Hi Arthur,
On 2024-02-07 at 17:17:15 -0300, Arthur Grillo wrote:
> Create a benchmark for the VKMS driver. Use a KMS layout with deliberate
> odd sizes to try to avoid alignment accidents and run it for FRAME_COUNT
> frames flipping framebuffers in each plane.
> 
> Link: https://lore.kernel.org/all/20240202214527.1d97c881@ferris.localdomain/
> Suggested-by: Pekka Paalanen <pekka.paala...@haloniitty.fi>
> Signed-off-by: Arthur Grillo <arthurgri...@riseup.net>
> ---
> This benchmark was suggested by Pekka Paalanen on [1] to better analyse
> possible performance regression on the Virtual Kernel Modesetting(VKMS)
> driver.
> 
> With this benchmark I was able to determine two performance regression:
> 
> - 322d716a3e8a ("drm/vkms: isolate pixel conversion functionality")
> - cc4fd2934d41 ("drm/vkms: Isolate writeback pixel conversion functions")

Could you move those explanations into commit description?

> 
> [1]: https://lore.kernel.org/all/20240202214527.1d97c881@ferris.localdomain/
> ---
>  benchmarks/meson.build   |   1 +
>  benchmarks/vkms_stress.c | 203 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 204 insertions(+)
> 
> diff --git a/benchmarks/meson.build b/benchmarks/meson.build
> index c451268bc44f..3aa66d6dffe2 100644
> --- a/benchmarks/meson.build
> +++ b/benchmarks/meson.build
> @@ -20,6 +20,7 @@ benchmark_progs = [
>       'kms_vblank',
>       'prime_lookup',
>       'vgem_mmap',
> +     'vkms_stress',
>  ]
>  
>  benchmarksdir = join_paths(libexecdir, 'benchmarks')
> diff --git a/benchmarks/vkms_stress.c b/benchmarks/vkms_stress.c
> new file mode 100644
> index 000000000000..b9128c208861
> --- /dev/null
> +++ b/benchmarks/vkms_stress.c
> @@ -0,0 +1,203 @@

Please add here at begin:
// SPDX-License-Identifier: MIT

> +/*
> + * Copyright © 2024 Arthur Grillo
> + *

Keep above copyright and delete below text, it is replaced by SPDX.

Regards,
Kamil

> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Arthur Grillo <arthurgri...@riseup.net>
> + *
> + */
> +
> +#include "igt.h"
> +
> +#define FRAME_COUNT 100
> +
> +struct rect_t {
> +     int x, y;
> +     int width, height;
> +};
> +
> +struct plane_t {
> +     igt_plane_t *base;
> +     struct rect_t rect;
> +     uint32_t format;
> +     struct igt_fb fbs[2];
> +};
> +
> +struct kms_t {
> +     struct plane_t primary;
> +     struct plane_t overlay_a;
> +     struct plane_t overlay_b;
> +     struct plane_t writeback;
> +};
> +
> +struct data_t {
> +     int fd;
> +     igt_display_t display;
> +     igt_output_t *wb_output;
> +     drmModeModeInfo *mode;
> +     struct kms_t kms;
> +};
> +
> +static void plane_create_fb(struct plane_t *plane, int fd, size_t index)
> +{
> +     igt_create_fb(fd, plane->rect.width, plane->rect.height,
> +                     plane->format, DRM_FORMAT_MOD_LINEAR,
> +                     &plane->fbs[index]);
> +}
> +
> +static void plane_create_color_fb(struct plane_t *plane, int fd, size_t 
> index, double r, double g,
> +                                double b)
> +{
> +     igt_create_color_fb(fd, plane->rect.width, plane->rect.height,
> +                     plane->format, DRM_FORMAT_MOD_LINEAR,
> +                     r, g, b,
> +                     &plane->fbs[index]);
> +}
> +
> +static void plane_setup(struct plane_t *plane, int index)
> +{
> +     igt_plane_set_size(plane->base, plane->rect.width, plane->rect.height);
> +     igt_plane_set_position(plane->base, plane->rect.x, plane->rect.y);
> +     igt_plane_set_fb(plane->base, &plane->fbs[index]);
> +}
> +
> +static void gen_fbs(struct data_t *data)
> +{
> +     struct kms_t *kms = &data->kms;
> +     drmModeModeInfo *mode = igt_output_get_mode(data->wb_output);
> +
> +     for (int i = 0; i < 2; i++) {
> +             plane_create_color_fb(&kms->primary, data->fd, i, !i, i, i);
> +
> +             plane_create_color_fb(&kms->overlay_a, data->fd, i, i, !i, i);
> +
> +             plane_create_color_fb(&kms->overlay_b, data->fd, i, i, i, !i);
> +
> +             kms->writeback.rect.width = mode->hdisplay;
> +             kms->writeback.rect.height = mode->vdisplay;
> +             plane_create_fb(&kms->writeback, data->fd, i);
> +     }
> +}
> +
> +static igt_output_t *find_wb_output(struct data_t *data)
> +{
> +     for (int i = 0; i < data->display.n_outputs; i++) {
> +             igt_output_t *output = &data->display.outputs[i];
> +
> +             if (output->config.connector->connector_type != 
> DRM_MODE_CONNECTOR_WRITEBACK)
> +                     continue;
> +
> +             return output;
> +
> +     }
> +
> +     return NULL;
> +}
> +
> +static struct kms_t default_kms = {
> +     .primary = {
> +             .rect = {
> +                     .x = 101, .y = 0,
> +                     .width = 3639, .height = 2161,
> +             },
> +             .format = DRM_FORMAT_XRGB8888,
> +     },
> +     .overlay_a = {
> +             .rect = {
> +                     .x = 201, .y = 199,
> +                     .width = 3033, .height = 1777,
> +             },
> +             .format = DRM_FORMAT_XRGB16161616,
> +     },
> +     .overlay_b = {
> +             .rect = {
> +                     .x = 1800, .y = 250,
> +                     .width = 1507, .height = 1400,
> +             },
> +             .format = DRM_FORMAT_ARGB8888,
> +     },
> +     .writeback = {
> +             .rect = {
> +                     .x = 0, .y = 0,
> +                     // Size is to be determined at runtime
> +             },
> +             .format = DRM_FORMAT_XRGB8888,
> +     },
> +};
> +
> +
> +igt_simple_main
> +{
> +     struct data_t data;
> +     enum pipe pipe = PIPE_NONE;
> +
> +     data.kms = default_kms;
> +
> +     data.fd = drm_open_driver_master(DRIVER_ANY);
> +
> +     igt_display_require(&data.display, data.fd);
> +
> +     kmstest_set_vt_graphics_mode();
> +
> +     igt_display_require(&data.display, data.fd);
> +     igt_require(data.display.is_atomic);
> +
> +     igt_display_require_output(&data.display);
> +
> +     igt_require(data.wb_output);
> +     igt_display_reset(&data.display);
> +
> +     data.wb_output = find_wb_output(&data);
> +
> +     for_each_pipe(&data.display, pipe) {
> +             igt_debug("Selecting pipe %s to %s\n",
> +                       kmstest_pipe_name(pipe),
> +                       igt_output_name(data.wb_output));
> +             igt_output_set_pipe(data.wb_output, pipe);
> +     }
> +
> +     igt_display_commit_atomic(&data.display, DRM_MODE_ATOMIC_ALLOW_MODESET, 
> NULL);
> +
> +     gen_fbs(&data);
> +
> +     data.kms.primary.base = igt_output_get_plane_type(data.wb_output, 
> DRM_PLANE_TYPE_PRIMARY);
> +     data.kms.overlay_a.base = 
> igt_output_get_plane_type_index(data.wb_output,
> +                                                               
> DRM_PLANE_TYPE_OVERLAY, 0);
> +     data.kms.overlay_b.base = 
> igt_output_get_plane_type_index(data.wb_output,
> +                                                               
> DRM_PLANE_TYPE_OVERLAY, 1);
> +
> +     for (int i = 0; i < FRAME_COUNT; i++) {
> +             int fb_index = i % 2;
> +
> +             plane_setup(&data.kms.primary, fb_index);
> +
> +             plane_setup(&data.kms.overlay_a, fb_index);
> +
> +             plane_setup(&data.kms.overlay_b, fb_index);
> +
> +             igt_output_set_writeback_fb(data.wb_output, 
> &data.kms.writeback.fbs[fb_index]);
> +
> +             igt_display_commit2(&data.display, COMMIT_ATOMIC);
> +     }
> +
> +     igt_display_fini(&data.display);
> +     drm_close_driver(data.fd);
> +}
> 
> ---
> base-commit: c58c5fb6aa1cb7d3627a15e364816a7a2add9edc
> change-id: 20240207-bench-393789eaba47
> 
> Best regards,
> -- 
> Arthur Grillo <arthurgri...@riseup.net>
> 

Reply via email to