Re: [PATCH i-g-t 11/15] lib/kms: Use for_each_crtc_with_valid_output(), part 1

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

On Fri, 30 Jan 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Convert some uses of for_each_pipe_with_valid_output()
> to for_each_crtc_with_valid_output(). Take care of the
> cases where we (presumably) still have to keep some
> kind of 'data.pipe' thingy still up to date.

*tests/kms

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


>
> Eventually we should probably replace 'data.pipe' with
> 'data.crtc' but I didn't want to go that far here.
>
> Done with cocci:
>  #include "scripts/iterators.cocci"
>
> @change@
> iterator name for_each_pipe_with_valid_output;
> iterator name for_each_crtc_with_valid_output;
> typedef igt_crtc_t;
> identifier PIPE;
> expression DISPLAY, OUTPUT, X;
> @@
> - for_each_pipe_with_valid_output(DISPLAY, X.PIPE, OUTPUT)
> + for_each_crtc_with_valid_output(DISPLAY, crtc, OUTPUT)
> {
> + X.PIPE = crtc->pipe;
> <...
> (
> - igt_crtc_for_pipe(..., X.PIPE)
> + crtc
> |
> - kmstest_pipe_name(X.PIPE)
> + igt_crtc_name(crtc)
> |
> - X.PIPE
> + crtc->pipe
> )
> ...>
> }
>
> @depends on change@
> identifier FUNC;
> @@
> FUNC(...)
> {
> + igt_crtc_t *crtc;
> <+...
> for_each_crtc_with_valid_output(...) { ... }
> ...+>
> }
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/intel/kms_psr2_sf.c |  7 +++++--
>  tests/kms_lease.c         | 15 +++++++++------
>  tests/kms_vblank.c        | 26 +++++++++++++++++---------
>  3 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/tests/intel/kms_psr2_sf.c b/tests/intel/kms_psr2_sf.c
> index f42863874e5e..e23999544370 100644
> --- a/tests/intel/kms_psr2_sf.c
> +++ b/tests/intel/kms_psr2_sf.c
> @@ -1175,6 +1175,7 @@ static void run_plane_update_continuous(data_t data, int i, int coexist_features
>  
>  int igt_main()
>  {
> +	igt_crtc_t *crtc;
>  	bool output_supports_pr_psr2_sel_fetch = false;
>  	bool pr_psr2_sel_fetch_supported = false;
>  	data_t data = {};
> @@ -1219,11 +1220,13 @@ int igt_main()
>  		igt_info("Big framebuffer size %dx%d\n",
>  			 data.big_fb_width, data.big_fb_height);
>  
> -		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
> +		for_each_crtc_with_valid_output(&data.display, crtc,
> +						data.output) {
> +			data.pipe = crtc->pipe;
>  			coexist_features[n_pipes] = 0;
>  			output_supports_pr_psr2_sel_fetch = check_pr_psr2_sel_fetch_support(&data);
>  			if (output_supports_pr_psr2_sel_fetch) {
> -				pipes[n_pipes] = data.pipe;
> +				pipes[n_pipes] = crtc->pipe;
>  				outputs[n_pipes] = data.output;
>  
>  				if (is_dsc_supported_by_sink(data.drm_fd, data.output))
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index 6d45d041b5b4..951159e97779 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -1240,6 +1240,7 @@ static void lease_uevent(data_t *data)
>  
>  int igt_main()
>  {
> +	igt_crtc_t *crtc;
>  	data_t data;
>  	igt_output_t *output;
>  	igt_display_t *display = &data.master.display;
> @@ -1288,21 +1289,23 @@ int igt_main()
>  
>  			igt_describe(f->desc);
>  			igt_subtest_with_dynamic_f("%s", f->name) {
> -				for_each_pipe_with_valid_output(display, data.pipe, output) {
> +				for_each_crtc_with_valid_output(display, crtc,
> +								output) {
> +					data.pipe = crtc->pipe;
>  					igt_display_reset(display);
>  
>  					igt_output_set_crtc(output,
> -							    igt_crtc_for_pipe(output->display, data.pipe));
> +							    crtc);
>  					if (!intel_pipe_output_combo_valid(display))
>  						continue;
>  
> -					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe),
> +					igt_dynamic_f("pipe-%s-%s",
> +						      igt_crtc_name(crtc),
>  						      igt_output_name(output)) {
> -						data.crtc_id = igt_crtc_for_pipe(display,
> -										 data.pipe)->crtc_id;
> +						data.crtc_id = crtc->crtc_id;
>  						data.connector_id = output->id;
>  						data.plane_id =
> -							igt_crtc_get_plane_type(igt_crtc_for_pipe(&data.master.display, data.pipe),
> +							igt_crtc_get_plane_type(crtc,
>  										DRM_PLANE_TYPE_PRIMARY)->drm_plane->plane_id;
>  						f->func(&data);
>  					}
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index ed72c3576784..1e4ccd83b554 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -625,6 +625,7 @@ const char *help_str =
>  
>  int igt_main_args("e", NULL, help_str, opt_handler, NULL)
>  {
> +	igt_crtc_t *crtc;
>  	int fd;
>  	data_t data;
>  
> @@ -642,11 +643,14 @@ int igt_main_args("e", NULL, help_str, opt_handler, NULL)
>  
>  	igt_describe("Negative test for vblank request.");
>  	igt_subtest_with_dynamic("invalid") {
> -		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
> -			if (!pipe_output_combo_valid(&data.display, data.pipe, data.output))
> +		for_each_crtc_with_valid_output(&data.display, crtc,
> +						data.output) {
> +			data.pipe = crtc->pipe;
> +			if (!pipe_output_combo_valid(&data.display, crtc->pipe, data.output))
>  				continue;
>  
> -			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
> +			igt_dynamic_f("pipe-%s-%s", igt_crtc_name(crtc),
> +				      data.output->name)
>  				invalid_subtest(&data, fd);
>  			/* one pipe/output combination is enough */
>  				break;
> @@ -655,17 +659,21 @@ int igt_main_args("e", NULL, help_str, opt_handler, NULL)
>  
>  	igt_describe("Check the vblank and flip events works with given crtc id.");
>  	igt_subtest_with_dynamic("crtc-id") {
> -		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
> -			if (!pipe_output_combo_valid(&data.display, data.pipe, data.output))
> +		for_each_crtc_with_valid_output(&data.display, crtc,
> +						data.output) {
> +			data.pipe = crtc->pipe;
> +			if (!pipe_output_combo_valid(&data.display, crtc->pipe, data.output))
>  				continue;
>  
> -			if (!all_pipes && data.pipe != active_pipes[0] &&
> -					  data.pipe != active_pipes[last_pipe]) {
> -				igt_info("Skipping pipe %s\n", kmstest_pipe_name(data.pipe));
> +			if (!all_pipes && crtc->pipe != active_pipes[0] &&
> +					  crtc->pipe != active_pipes[last_pipe]) {
> +				igt_info("Skipping pipe %s\n",
> +					 igt_crtc_name(crtc));
>  				continue;
>  			}
>  
> -			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
> +			igt_dynamic_f("pipe-%s-%s", igt_crtc_name(crtc),
> +				      data.output->name)
>  				crtc_id_subtest(&data, fd);
>  		}
>  	}

-- 
Jani Nikula, Intel