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

Kory Maincent
2026-02-25T10:20:09Z

On Wed, 25 Feb 2026 11:00:27 +0100
Louis Chauvet <louis.chauvet@bootlin.com> wrote:

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

Oh ok legit.
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering