Re: [PATCH i-g-t v7 41/48] lib/igt_dp: Create generic helpers for DP information

Kory Maincent
2026-02-23T15:56:37Z

On Mon, 23 Feb 2026 13:09:48 +0100
Louis Chauvet <louis.chauvet@bootlin.com> wrote:

> With the introduction of unigraf tests, there is no reason to limit the
> implementation to intel. Create easly extendable generic functions to
> help supporting multiple GPU.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
> lib/igt_dp.c | 177
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/igt_dp.h |
> 30 ++++++++++ lib/meson.build | 1 +
> 3 files changed, 208 insertions(+)
>
> diff --git a/lib/igt_dp.c b/lib/igt_dp.c
> new file mode 100644
> index 000000000000..0920a43af5e9
> --- /dev/null
> +++ b/lib/igt_dp.c
> @@ -0,0 +1,177 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Google
> + *
> + * Authors:
> + * Louis Chauvet <louis.chauvet@bootlin.com>
> + */
> +
> +#include "drmtest.h"
> +#include "i915/i915_dp.h"
> +#include "igt_amd.h"

I think this header is useless, there is no amd function call here.

> +#include "igt_core.h"
> +#include "igt_kms.h"
> +#include <stdint.h>
> +#include "igt_dp.h"

stdint include should be at the top.
We often put system header before local header.

> +
> +/**
> + * igt_dp_get_current_link_rate: Get current link rate on a display port
> + * @drm_fd: DRM file descriptor
> + * @output: igt_output_t object representing the display port
> + *
> + * Returns:
> + * The current link rate in kb/s, or a negative error code on failure.
> + */
> +int igt_dp_get_current_link_rate(int drm_fd, igt_output_t *output)
> +{
> + if (is_intel_device(drm_fd))
> + /*
> + * i915_dp_get_current_link_rate returns the value in tens
> of kb/s because
> + * that what the kernel uses. Convert it to kb/s to have a
> sane unit...
> + */
> + return i915_dp_get_current_link_rate(drm_fd, output) * 10;
> +
> + igt_assert_f(false, "Current drm device is not able to report used
> link rate\n");
> + return -EINVAL;
> +}

I usually do the other way around:
if (!is_intel_device(...)) {
igt_assert(...);
return -EINVAL
}

return i915_dp_get_current_link_rate(drm_fd, output) * 10;

but well that's only my coding style I am also ok with that.

...

> +void igt_dp_wait_pending_retrain(int drm_fd, igt_output_t *output)
> +{
> + double timeout = igt_default_display_detect_timeout();
> + struct timespec start, now;
> + double elapsed;
> +
> + clock_gettime(CLOCK_MONOTONIC, &start);
> +
> + while (1) {
> + if (!igt_dp_get_pending_retrain(drm_fd, output))
> + return;
> +
> + clock_gettime(CLOCK_MONOTONIC, &now);
> + elapsed = (now.tv_sec - start.tv_sec) +
> + (now.tv_nsec - start.tv_nsec) / 1e9;
> +
> + if (elapsed >= timeout)
> + break;
> + usleep(10);

10us? I think you wanted 10ms here.

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering