Re: [PATCH i-g-t 01/12] igt/kms: Nuke igt_pipe_is_prop_changed() and igt_pipe_set_prop_value()

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 20 Jan 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Remove the pipe based igt_pipe_is_prop_changed() and
> igt_pipe_set_prop_value() wrappers and just use the crtc
> based variants directly. One step towards eliminating
> pipe usage from tests.
>
> Done with cocci:
>  #include "scripts/iterators.cocci"
>
> @@
> @@
> - igt_pipe_is_prop_changed(...) { ... }
>
> @@
> @@
> - igt_pipe_set_prop_value(...) { ... }
>
> @@
> expression DISPLAY, PIPE;
> @@
> (
> - igt_pipe_is_prop_changed(DISPLAY, PIPE
> + igt_pipe_obj_is_prop_changed(igt_crtc_for_pipe(DISPLAY, PIPE)
> 	,...)
> |
> - igt_pipe_set_prop_value(DISPLAY, PIPE
> + igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(DISPLAY, PIPE)
> 	,...)
> )
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  lib/igt_kms.h                                 | 32 -------------------
>  tests/amdgpu/amd_freesync_video_mode.c        |  5 +--
>  .../kms_chamelium_sharpness_filter.c          | 10 +++---
>  tests/intel/kms_pm_dc.c                       |  5 +--
>  tests/intel/kms_sharpness_filter.c            |  6 ++--
>  tests/kms_atomic_transition.c                 |  2 +-
>  tests/kms_content_protection.c                |  8 ++---
>  tests/kms_vrr.c                               | 11 ++++---
>  8 files changed, 26 insertions(+), 53 deletions(-)
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a271c94a97fd..b848ca2eb973 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1067,21 +1067,6 @@ static inline bool igt_pipe_obj_is_prop_changed(igt_crtc_t *pipe_obj,
>  	return pipe_obj->changed & (1 << prop);
>  }
>  
> -/**
> - * igt_pipe_is_prop_changed:
> - * @pipe: Pipe object to check.
> - * @prop: Property to check.
> - *
> - * Check whether a given @prop changed for the @pipe.
> - */
> -static inline bool igt_pipe_is_prop_changed(igt_display_t *display,
> -					    enum pipe pipe,
> -					    enum igt_atomic_crtc_properties prop)
> -{
> -	return igt_pipe_obj_is_prop_changed(igt_crtc_for_pipe(display, pipe),
> -					    prop);
> -}
> -
>  /**
>   * igt_pipe_obj_set_prop_changed:
>   * @pipe_obj: Pipe object to check.
> @@ -1124,23 +1109,6 @@ static inline void igt_pipe_obj_set_prop_value(igt_crtc_t *pipe_obj,
>  	igt_pipe_obj_set_prop_changed(pipe_obj, prop);
>  }
>  
> -/**
> - * igt_pipe_set_prop_value:
> - * @pipe: Pipe to check.
> - * @prop: Property to check.
> - * @value: Value to set.
> - *
> - * Sets the given @prop with the @value for the @pipe.
> - */
> -static inline void igt_pipe_set_prop_value(igt_display_t *display,
> -					   enum pipe pipe,
> -					   enum igt_atomic_crtc_properties prop,
> -					   uint64_t value)
> -{
> -	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(display, pipe), prop,
> -				    value);
> -}
> -
>  extern bool igt_pipe_obj_try_prop_enum(igt_crtc_t *pipe,
>  				       enum igt_atomic_crtc_properties prop,
>  				       const char *val);
> diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
> index c1a3e8161304..812212ac5f2b 100644
> --- a/tests/amdgpu/amd_freesync_video_mode.c
> +++ b/tests/amdgpu/amd_freesync_video_mode.c
> @@ -548,8 +548,9 @@ static bool has_vrr(igt_output_t *output)
>  /* Toggles variable refresh rate on the pipe. */
>  static void set_vrr_on_pipe(data_t *data, enum pipe pipe, bool enabled)
>  {
> -	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED,
> -				enabled);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, pipe),
> +				    IGT_CRTC_VRR_ENABLED,
> +				    enabled);
>  	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  }
>  
> diff --git a/tests/chamelium/kms_chamelium_sharpness_filter.c b/tests/chamelium/kms_chamelium_sharpness_filter.c
> index 6f161660b043..fb6e95a92b5e 100644
> --- a/tests/chamelium/kms_chamelium_sharpness_filter.c
> +++ b/tests/chamelium/kms_chamelium_sharpness_filter.c
> @@ -53,15 +53,15 @@ static bool pipe_output_combo_valid(data_t *data, enum pipe pipe)
>  
>  static void set_filter_strength_on_pipe(data_t *data)
>  {
> -	igt_pipe_set_prop_value(&data->display, data->pipe_id,
> -				IGT_CRTC_SHARPNESS_STRENGTH,
> -				data->filter_strength);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, data->pipe_id),
> +				    IGT_CRTC_SHARPNESS_STRENGTH,
> +				    data->filter_strength);
>  }
>  
>  static void reset_filter_strength_on_pipe(data_t *data)
>  {
> -	igt_pipe_set_prop_value(&data->display, data->pipe_id,
> -				IGT_CRTC_SHARPNESS_STRENGTH, 0);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, data->pipe_id),
> +				    IGT_CRTC_SHARPNESS_STRENGTH, 0);
>  }
>  
>  static void paint_image(igt_fb_t *fb)
> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
> index 5e59ba6e03a3..9055e66a2ffa 100644
> --- a/tests/intel/kms_pm_dc.c
> +++ b/tests/intel/kms_pm_dc.c
> @@ -663,8 +663,9 @@ static void test_deep_pkgc_state(data_t *data)
>  				 * TODO: Add check for vmin = vmax = flipline if VRR enabled
>  				 * when KMD allows for such capability.
>  				 */
> -				igt_pipe_set_prop_value(display, pipe,
> -							IGT_CRTC_VRR_ENABLED, false);
> +				igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(display, pipe),
> +							    IGT_CRTC_VRR_ENABLED,
> +							    false);
>  				igt_assert(igt_display_try_commit_atomic(display,
>  									 DRM_MODE_ATOMIC_ALLOW_MODESET,
>  									 NULL) == 0);
> diff --git a/tests/intel/kms_sharpness_filter.c b/tests/intel/kms_sharpness_filter.c
> index 06ceb640a976..ffc43dbaf129 100644
> --- a/tests/intel/kms_sharpness_filter.c
> +++ b/tests/intel/kms_sharpness_filter.c
> @@ -173,9 +173,9 @@ typedef struct {
>  
>  static void set_filter_strength_on_pipe(data_t *data)
>  {
> -	igt_pipe_set_prop_value(&data->display, data->pipe_id,
> -				IGT_CRTC_SHARPNESS_STRENGTH,
> -				data->filter_strength);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, data->pipe_id),
> +				    IGT_CRTC_SHARPNESS_STRENGTH,
> +				    data->filter_strength);
>  }
>  
>  static bool has_scaling_mode(igt_output_t *output)
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 4631d54725db..6aa3439eedae 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -829,7 +829,7 @@ static unsigned set_combinations(data_t *data, unsigned mask, struct igt_fb *fb)
>  		drmModeModeInfo *mode = NULL;
>  
>  		if (!(mask & (1 << pipe))) {
> -			if (igt_pipe_is_prop_changed(&data->display, pipe, IGT_CRTC_ACTIVE)) {
> +			if (igt_pipe_obj_is_prop_changed(igt_crtc_for_pipe(&data->display, pipe), IGT_CRTC_ACTIVE)) {
>  				event_mask |= 1 << pipe;
>  				igt_plane_set_fb(plane, NULL);
>  			}
> diff --git a/tests/kms_content_protection.c b/tests/kms_content_protection.c
> index c1a84c63f791..f7896592b3ff 100644
> --- a/tests/kms_content_protection.c
> +++ b/tests/kms_content_protection.c
> @@ -484,12 +484,12 @@ static void test_content_protection_on_output(igt_output_t *output,
>  		test_cp_lic(output);
>  
>  	if (data.cp_tests & CP_DPMS) {
> -		igt_pipe_set_prop_value(display, pipe,
> -					IGT_CRTC_ACTIVE, 0);
> +		igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(display, pipe),
> +					    IGT_CRTC_ACTIVE, 0);
>  		igt_display_commit2(display, commit_style);
>  
> -		igt_pipe_set_prop_value(display, pipe,
> -					IGT_CRTC_ACTIVE, 1);
> +		igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(display, pipe),
> +					    IGT_CRTC_ACTIVE, 1);
>  		igt_display_commit2(display, commit_style);
>  
>  		ret = wait_for_prop_value(output, CP_ENABLED,
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 0e54672a7d83..f05b3b87ad57 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -309,8 +309,9 @@ static bool vrr_capable(igt_output_t *output)
>  static void set_vrr_on_pipe(data_t *data, enum pipe pipe,
>  			    bool need_modeset, bool enabled)
>  {
> -	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED,
> -				enabled);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, pipe),
> +				    IGT_CRTC_VRR_ENABLED,
> +				    enabled);
>  
>  	igt_assert(igt_display_try_commit_atomic(&data->display,
>  						 need_modeset ? DRM_MODE_ATOMIC_ALLOW_MODESET : 0,
> @@ -405,7 +406,8 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>  	/* Clear vrr_enabled state before enabling it, because
>  	 * it might be left enabled if the previous test fails.
>  	 */
> -	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, 0);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, pipe),
> +				    IGT_CRTC_VRR_ENABLED, 0);
>  
>  	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  }
> @@ -968,7 +970,8 @@ test_cmrr(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>  
>  static void test_cleanup(data_t *data, enum pipe pipe, igt_output_t *output)
>  {
> -	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, false);
> +	igt_pipe_obj_set_prop_value(igt_crtc_for_pipe(&data->display, pipe),
> +				    IGT_CRTC_VRR_ENABLED, false);
>  
>  	if (data->primary)
>  		igt_plane_set_fb(data->primary, NULL);

-- 
Jani Nikula, Intel