[Intel-gfx] [PATCH i-g-t] kms_atomic: Measure speed of some plane ioctls
Chris Wilson
chris at chris-wilson.co.uk
Tue Apr 28 05:56:53 PDT 2015
On Tue, Apr 28, 2015 at 01:46:23PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Measures DRM_IOCTL_MODE_SETCRTC and DRM_IOCTL_MODE_SETPLANE as proxy for
> drm_atomic_helper_update_plane if I got it right.
>
> Discovered some slow cursor updates (1.6ms) so needed something to test
> different kernel configs etc.
>
> v2:
> * Move to a test case and fail if ioctl takes more than 1ms. (Chris Wilson)
> * Add some kerneldoc to satisfy the form. (Thomas Wood)
>
> v3:
> * Simplified a bit for better clarity. (Chris Wilson)
> * Removed internal use comment. (Thomas Wood)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> lib/igt_kms.c | 19 +++--
> lib/igt_kms.h | 5 ++
> tests/.gitignore | 1 +
> tests/Makefile.sources | 1 +
> tests/kms_atomic.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 214 insertions(+), 5 deletions(-)
> create mode 100644 tests/kms_atomic.c
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 0665d70..308db1e 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1537,14 +1537,23 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
> }
>
>
> -/*
> +/**
> + * igt_plane_commit:
> + * @plane: plane to commit
> + * @output: output
> + * @s: commit style, legacy or univeral
> + * @fail_on_error: fail or just return an error code
> + *
> * Commit position and fb changes to a plane. The value of @s will determine
> * which API is used to do the programming.
> + *
> + * This is a low-level function and you are recommened to rather use something
> + * from the igt_display_commit family.
> */
> -static int igt_plane_commit(igt_plane_t *plane,
> - igt_output_t *output,
> - enum igt_commit_style s,
> - bool fail_on_error)
> +int igt_plane_commit(igt_plane_t *plane,
> + igt_output_t *output,
> + enum igt_commit_style s,
> + bool fail_on_error)
> {
> if (plane->is_cursor && s == COMMIT_LEGACY) {
> return igt_cursor_commit_legacy(plane, output, fail_on_error);
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 09c08aa..a4dd30b 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -267,6 +267,11 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>
> void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
> +int igt_plane_commit(igt_plane_t *plane,
> + igt_output_t *output,
> + enum igt_commit_style s,
> + bool fail_on_error);
> +
> #define for_each_connected_output(display, output) \
> for (int i__ = 0; i__ < (display)->n_outputs; i__++) \
> if ((output = &(display)->outputs[i__]), output->valid)
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 796e330..7b479d3 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -124,6 +124,7 @@ gen3_render_tiledy_blits
> gen7_forcewake_mt
> kms_3d
> kms_addfb
> +kms_atomic
> kms_cursor_crc
> kms_fbc_crc
> kms_fence_pin_leak
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 4cbc50d..feb84eb 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -59,6 +59,7 @@ TESTS_progs_M = \
> gem_userptr_blits \
> gem_write_read_ring_switch \
> kms_addfb \
> + kms_atomic \
> kms_cursor_crc \
> kms_fbc_crc \
> kms_flip \
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> new file mode 100644
> index 0000000..5510c9f
> --- /dev/null
> +++ b/tests/kms_atomic.c
> @@ -0,0 +1,193 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * 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.
> + *
> + */
> +
> +#include <math.h>
> +#include <sys/time.h>
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "igt_core.h"
> +#include "intel_chipset.h"
> +
> +typedef struct {
> + int gfx_fd;
> + igt_display_t display;
> + struct igt_fb fb;
> + struct igt_fb fb_modeset;
> +} data_t;
> +
> +
> +static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
> + igt_plane_t *plane, enum igt_commit_style commit)
> +{
> + drmModeModeInfo *mode;
> + igt_display_t *display = &data->display;
> + int fb_id, fb_modeset_id;
> + unsigned int w, h;
> + uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE;
> + uint32_t pixel_format = DRM_FORMAT_XRGB8888;
> + igt_plane_t *primary;
> +
> + igt_output_set_pipe(output, pipe);
> +
> + mode = igt_output_get_mode(output);
> +
> + w = mode->hdisplay;
> + h = mode->vdisplay;
> +
> + fb_modeset_id = igt_create_fb(data->gfx_fd,
> + w, h,
> + pixel_format,
> + tiling,
> + &data->fb_modeset);
> + igt_assert(fb_modeset_id);
> +
> + /*
> + * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the
> + * setplane without a modeset. So, to be able to call
> + * igt_display_commit and ultimately setcrtc to do the first modeset,
> + * we create an fb covering the crtc and call commit
> + */
> +
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + igt_plane_set_fb(primary, &data->fb_modeset);
> + igt_display_commit(display);
> +
> + fb_id = igt_create_fb(data->gfx_fd,
> + w, h,
> + pixel_format,
> + tiling,
> + &data->fb);
> + igt_assert(fb_id);
> +
> + igt_plane_set_fb(plane, &data->fb);
> +
> + igt_display_commit2(display, commit);
> +}
> +
> +static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
> +{
> + igt_display_t *display = &data->display;
> +
> + igt_remove_fb(data->gfx_fd, &data->fb);
> + igt_remove_fb(data->gfx_fd, &data->fb_modeset);
> +
> + /* XXX: see the note in prepare_crtc() */
> + if (!plane->is_primary) {
> + igt_plane_t *primary;
> +
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + igt_plane_set_fb(primary, NULL);
> + }
> +
> + igt_plane_set_fb(plane, NULL);
> + igt_output_set_pipe(output, PIPE_ANY);
> +
> + igt_display_commit(display);
> +}
> +
> +static double elapsed(const struct timeval *start,
> + const struct timeval *end,
> + int loop)
> +{
> + return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - start->tv_usec))/loop;
> +}
> +
> +static void test_commit_speed(data_t *data, enum igt_commit_style commit)
> +{
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> + enum pipe pipe;
> + unsigned int i;
> + const unsigned int loops = 20000;
> + struct timeval start, end;
> + double e;
> +
> + if (commit == COMMIT_UNIVERSAL)
> + igt_require(data->display.has_universal_planes);
> +
> + for_each_connected_output(display, output) {
> + for_each_pipe(display, pipe) {
> + igt_plane_t *plane;
> +
> + igt_output_set_pipe(output, pipe);
> + plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> +
> + prepare_crtc(data, output, pipe, plane, commit);
> +
> + gettimeofday(&start, NULL);
> + for (i = loops; i > 0; i--) {
> + /*
> + * Trick igt_plane_commit into calling either
> + * DRM_IOCTL_MODE_SETCRTC or
> + * DRM_IOCTL_MODE_SETPLANE, depending on the
> + * commit mode.
> + *
> + * igt_plane_commit is a low-level API which
> + * basically just calls one of the two commit
> + * paths.
> + */
> + plane->position_changed = true;
> + igt_plane_commit(plane, output, commit, true);
> + }
> + gettimeofday(&end, NULL);
> + e = elapsed(&start, &end, loops);
> + igt_info("Pipe: %u Plane: %u Time: %7.3fµs\n",
> + pipe, plane->index, e);
> + igt_require(e < 1000); /* 1ms, just so. */
> +
> + cleanup_crtc(data, output, plane);
> +
> + break;
Test each pipe, otherwise people will only fix bugs on pipe 0! From
experience, switching CRTCs tends to ellicit new ang exciting bugs.
Otherwise, lgtm.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx
mailing list