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

Louis Chauvet
2026-02-25T09:59:58Z

On 2/23/26 16:56, Kory Maincent wrote:
> 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.

The goal is to be able to easly add amd, qualcom, whatever like this:

if (is_intel())
return intel_...
if (is_amd())
return amd_...
...

> ...
>
>> +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.

Yes, I don't know how to count :-)

> Regards,