Re: [PATCH v2 31/34] drm/i915/kunit: Export link training and caps funcs for testing

On Wed, 01 Jul 2026, Imre Deak <[email protected]> wrote:
> Export the link caps and link training helpers needed by the DP link
> KUnit tests.
>
> Use test ops tables instead of exporting the helpers directly, avoiding
> symbol name collisions between the i915 and xe builds of the shared
> display code.

Okay, so this one slipped through and was merged.

I understand it took time and effort to figure out how to make this work
for both i915 and xe... but frankly, this is now an awful hack that
*must* be undone and refactored and fixed to work with the future
display module.

We can't have a single #ifdef I915.

I don't even see the real benefit for testing this for both i915 and xe,
separately. One should've been enough.


BR,
Jani.

>
> Signed-off-by: Imre Deak <[email protected]>
> ---
>  .../gpu/drm/i915/display/intel_dp_link_caps.c | 29 +++++++++++++++
>  .../gpu/drm/i915/display/intel_dp_link_caps.h | 37 +++++++++++++++++++
>  .../drm/i915/display/intel_dp_link_training.c | 36 +++++++++++++++++-
>  .../drm/i915/display/intel_dp_link_training.h | 31 ++++++++++++++++
>  .../i915/display/tests/intel_dp_link_test.c   | 17 +++++++++
>  5 files changed, 148 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_caps.c b/drivers/gpu/drm/i915/display/intel_dp_link_caps.c
> index 76b7c0fc90115..7b6cc6055da82 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_caps.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_caps.c
> @@ -1281,3 +1281,32 @@ void intel_dp_link_caps_cleanup(struct intel_dp_link_caps *link_caps)
>  {
>  	kfree(link_caps);
>  }
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
> +
> +#define __INIT_MEMBER(__name, __fn) \
> +	.__name = __fn,
> +
> +#define INTEL_DP_LINK_CAPS_TEST_OPS_INIT \
> +	INTEL_DP_LINK_CAPS_TEST_OPS_MEMBERS(__INIT_MEMBER)
> +
> +#ifdef I915
> +
> +const struct intel_dp_link_caps_test_ops i915_display_dp_link_caps_test_ops = {
> +	INTEL_DP_LINK_CAPS_TEST_OPS_INIT
> +};
> +EXPORT_SYMBOL(i915_display_dp_link_caps_test_ops);
> +
> +#else
> +
> +const struct intel_dp_link_caps_test_ops intel_display_dp_link_caps_test_ops = {
> +	INTEL_DP_LINK_CAPS_TEST_OPS_INIT
> +};
> +EXPORT_SYMBOL(intel_display_dp_link_caps_test_ops);
> +
> +#endif	/* I915 */
> +
> +#undef INTEL_DP_LINK_CAPS_TEST_OPS_INIT
> +#undef __INIT_MEMBER
> +
> +#endif	/* CONFIG_KUNIT */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_caps.h b/drivers/gpu/drm/i915/display/intel_dp_link_caps.h
> index 56c585eb5a135..a0a88efb95463 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_caps.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_caps.h
> @@ -155,4 +155,41 @@ void intel_dp_link_caps_debugfs_add(struct intel_connector *connector);
>  struct intel_dp_link_caps *intel_dp_link_caps_init(struct intel_dp *intel_dp);
>  void intel_dp_link_caps_cleanup(struct intel_dp_link_caps *link_caps);
>  
> +#if IS_ENABLED(CONFIG_KUNIT)
> +
> +#define INTEL_DP_LINK_CAPS_TEST_OPS_MEMBERS(__X) \
> +	__X(connector_compute_order,	intel_dp_link_caps_connector_compute_order) \
> +	__X(connector_fallback_order,	intel_dp_link_caps_connector_fallback_order) \
> +	__X(iter_start,			intel_dp_link_caps_iter_start) \
> +	__X(iter_end,			intel_dp_link_caps_iter_end) \
> +	__X(set_max_limits,		intel_dp_link_caps_set_max_limits) \
> +	__X(get_max_limits,		intel_dp_link_caps_get_max_limits) \
> +	__X(get_max_bw_config,		intel_dp_link_caps_get_max_bw_config) \
> +	__X(reset_max_limits,		intel_dp_link_caps_reset_max_limits) \
> +	__X(disable_config,		intel_dp_link_caps_disable_config) \
> +	__X(update,			intel_dp_link_caps_update) \
> +	__X(init,			intel_dp_link_caps_init) \
> +	__X(cleanup,			intel_dp_link_caps_cleanup)
> +
> +#define __DECLARE_MEMBER(__name, __fn) \
> +	typeof(__fn) *__name;
> +
> +#define INTEL_DP_LINK_CAPS_TEST_OPS_DECLARE \
> +	INTEL_DP_LINK_CAPS_TEST_OPS_MEMBERS(__DECLARE_MEMBER)
> +
> +struct intel_dp_link_caps_test_ops {
> +	INTEL_DP_LINK_CAPS_TEST_OPS_DECLARE
> +};
> +
> +#undef INTEL_DP_LINK_CAPS_TEST_OPS_DECLARE
> +#undef __DECLARE_MEMBER
> +
> +#ifdef I915
> +extern const struct intel_dp_link_caps_test_ops i915_display_dp_link_caps_test_ops;
> +#else
> +extern const struct intel_dp_link_caps_test_ops intel_display_dp_link_caps_test_ops;
> +#endif	/* I915 */
> +
> +#endif	/* CONFIG_KUNIT */
> +
>  #endif /* __INTEL_DP_LINK_CAPS_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index a592bfab5ff0e..fa55664c9d98e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -21,6 +21,8 @@
>   * IN THE SOFTWARE.
>   */
>  
> +#include <kunit/visibility.h>
> +
>  #include <linux/debugfs.h>
>  #include <linux/iopoll.h>
>  
> @@ -1888,8 +1890,9 @@ static bool reduce_link_params(struct intel_dp *intel_dp, const struct intel_crt
>  	return new_found;
>  }
>  
> -static int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> -						   const struct intel_crtc_state *crtc_state)
> +VISIBLE_IF_KUNIT
> +int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> +					    const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	struct intel_dp_link_caps *link_caps = intel_dp->link.caps;
> @@ -2813,3 +2816,32 @@ void intel_dp_link_training_cleanup(struct intel_dp_link_training *link_training
>  {
>  	kfree(link_training);
>  }
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
> +
> +#define __INIT_MEMBER(__name, __fn) \
> +	.__name = __fn,
> +
> +#define INTEL_DP_LINK_TRAINING_TEST_OPS_INIT \
> +	INTEL_DP_LINK_TRAINING_TEST_OPS_MEMBERS(__INIT_MEMBER)
> +
> +#ifdef I915
> +
> +const struct intel_dp_link_training_test_ops i915_display_dp_link_training_test_ops = {
> +	INTEL_DP_LINK_TRAINING_TEST_OPS_INIT
> +};
> +EXPORT_SYMBOL(i915_display_dp_link_training_test_ops);
> +
> +#else
> +
> +const struct intel_dp_link_training_test_ops intel_display_dp_link_training_test_ops = {
> +	INTEL_DP_LINK_TRAINING_TEST_OPS_INIT
> +};
> +EXPORT_SYMBOL(intel_display_dp_link_training_test_ops);
> +
> +#endif	/* I915 */
> +
> +#undef INTEL_DP_LINK_TRAINING_TEST_OPS_INIT
> +#undef __INIT_MEMBER
> +
> +#endif	/* CONFIG_KUNIT */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> index ef16fcabd6da9..581f2361fdfd5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> @@ -8,6 +8,8 @@
>  
>  #include <drm/display/drm_dp_helper.h>
>  
> +#include "intel_dp_link_caps.h"
> +
>  struct intel_atomic_state;
>  struct intel_connector;
>  struct intel_crtc_state;
> @@ -71,4 +73,33 @@ void intel_dp_link_training_reset(struct intel_dp_link_training *link_training);
>  struct intel_dp_link_training *intel_dp_link_training_init(struct intel_dp *intel_dp);
>  void intel_dp_link_training_cleanup(struct intel_dp_link_training *link_training);
>  
> +#if IS_ENABLED(CONFIG_KUNIT)
> +
> +int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> +					    const struct intel_crtc_state *crtc_state);
> +
> +#define INTEL_DP_LINK_TRAINING_TEST_OPS_MEMBERS(__X) \
> +	__X(get_fallback_values,	intel_dp_get_link_train_fallback_values)
> +
> +#define __DECLARE_MEMBER(__name, __fn) \
> +	typeof(__fn) *__name;
> +
> +#define INTEL_DP_LINK_TRAINING_TEST_OPS_DECLARE \
> +	INTEL_DP_LINK_TRAINING_TEST_OPS_MEMBERS(__DECLARE_MEMBER)
> +
> +struct intel_dp_link_training_test_ops {
> +	INTEL_DP_LINK_TRAINING_TEST_OPS_DECLARE
> +};
> +
> +#undef INTEL_DP_LINK_TRAINING_TEST_OPS_DECLARE
> +#undef __DECLARE_MEMBER
> +
> +#ifdef I915
> +extern const struct intel_dp_link_training_test_ops i915_display_dp_link_training_test_ops;
> +#else
> +extern const struct intel_dp_link_training_test_ops intel_display_dp_link_training_test_ops;
> +#endif	/* I915 */
> +
> +#endif	/* CONFIG_KUNIT */
> +
>  #endif /* __INTEL_DP_LINK_TRAINING_H__ */
> diff --git a/drivers/gpu/drm/i915/display/tests/intel_dp_link_test.c b/drivers/gpu/drm/i915/display/tests/intel_dp_link_test.c
> index aa5358c94839f..b77472e9bbe12 100644
> --- a/drivers/gpu/drm/i915/display/tests/intel_dp_link_test.c
> +++ b/drivers/gpu/drm/i915/display/tests/intel_dp_link_test.c
> @@ -17,6 +17,8 @@
>  #include "intel_connector.h"
>  #include "intel_display_core.h"
>  #include "intel_display_types.h"
> +#include "intel_dp_link_caps.h"
> +#include "intel_dp_link_training.h"
>  
>  struct test_ctx {
>  	struct {
> @@ -30,6 +32,9 @@ struct test_ctx {
>  		struct intel_crtc_state crtc_state;
>  	} dev;
>  
> +	const struct intel_dp_link_caps_test_ops *link_caps_ops;
> +	const struct intel_dp_link_training_test_ops *link_training_ops;
> +
>  	struct rnd_state rnd;
>  };
>  
> @@ -64,6 +69,8 @@ static int intel_dp_link_test_init(struct kunit *test)
>  	intel_dp = &dig_port->dp;
>  	intel_dp->attached_connector = &test_ctx.dev.connector;
>  
> +	intel_dp->link.caps = test_ctx.link_caps_ops->init(intel_dp);
> +
>  	test->priv = &test_ctx;
>  
>  	return 0;
> @@ -71,10 +78,20 @@ static int intel_dp_link_test_init(struct kunit *test)
>  
>  static void intel_dp_link_test_exit(struct kunit *test)
>  {
> +	struct test_ctx *ctx = test->priv;
> +
> +	ctx->link_caps_ops->cleanup(ctx->dev.dig_port.dp.link.caps);
>  }
>  
>  static int intel_dp_link_test_suite_init(struct kunit_suite *test_suite)
>  {
> +#ifdef I915
> +	test_ctx.link_caps_ops = &i915_display_dp_link_caps_test_ops;
> +	test_ctx.link_training_ops = &i915_display_dp_link_training_test_ops;
> +#else
> +	test_ctx.link_caps_ops = &intel_display_dp_link_caps_test_ops;
> +	test_ctx.link_training_ops = &intel_display_dp_link_training_test_ops;
> +#endif
>  	prandom_seed_state(&test_ctx.rnd, 0);
>  
>  	return 0;

-- 
Jani Nikula, Intel