RE: [PATCH i-g-t 1/2] tests/intel/kms_joiner_helper: Add igt_get_free_pipes_mask()

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

On Wed, 07 Oct 2026, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Wednesday, October 7, 2026 7:06 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; igt-dev@lists.freedesktop.org
>> Cc: B S, Karthik <karthik.b.s@intel.com>; Joshi, Kunal1
>> <kunal1.joshi@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
>> Subject: Re: [PATCH i-g-t 1/2] tests/intel/kms_joiner_helper: Add
>> igt_get_free_pipes_mask()
>> 
>> On Wed, 07 Oct 2026, Arun R Murthy <arun.r.murthy@intel.com> wrote:
>> > The joiner tests derive the set of usable pipes from the pipe count
>> > alone, which assumes every pipe is free. A pipe already driven by
>> > another client is then handed to a modeset, making the commit fail.
>> >
>> > Add igt_get_free_pipes_mask(), which gets the current CRTC usage via
>> > drmModeGetCrtcList() and translates it into a bitmask of hardware
>> > pipes.
>> >
>> > drmModeGetCrtcList() is a recent libdrm addition, so probe for it in
>> > meson and fall back to plain CRTC enumeration when it is missing.
>> >
>> > Assisted-by: LLM
>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> > ---
>> >  meson.build                     |  7 +++++
>> >  tests/intel/kms_joiner_helper.c | 66
>> > +++++++++++++++++++++++++++++++++++++++++
>> >  tests/intel/kms_joiner_helper.h |  1 +
>> >  3 files changed, 74 insertions(+)
>> >
>> > diff --git a/meson.build b/meson.build index
>> >
>> d7bbfd9698e5616ab777ac58128192ba1e0bec23..f41588c9d73361dbfd405de5
>> ea8f
>> > 764c6768a9c9 100644
>> > --- a/meson.build
>> > +++ b/meson.build
>> > @@ -129,6 +129,13 @@ endif
>> >
>> >  build_info += 'With libdrm: ' + ','.join(libdrm_info)
>> >
>> > +# drmModeGetCrtcList() is a recent libdrm addition; keep building
>> > +against # older libdrm by falling back to plain resource enumeration.
>> > +if cc.has_function('drmModeGetCrtcList', dependencies : libdrm,
>> > +		   prefix : '#include <xf86drmMode.h>')
>> > +	config.set('HAVE_DRMMODEGETCRTCLIST', 1) endif
>> > +
>> >  pciaccess = dependency('pciaccess', version : '>=0.10')  libkmod =
>> > dependency('libkmod')  # igt_procps.c reads /proc directly; libprocps
>> > and libproc2 are not needed.
>> > diff --git a/tests/intel/kms_joiner_helper.c
>> > b/tests/intel/kms_joiner_helper.c index
>> >
>> 36168030475c2725130e6c8ad924dcfb971a07f6..235e227fe9d82f598d0bd7df
>> 57ff
>> > 65d73d717ad0 100644
>> > --- a/tests/intel/kms_joiner_helper.c
>> > +++ b/tests/intel/kms_joiner_helper.c
>> > @@ -113,6 +113,72 @@ void
>> igt_set_all_master_pipes_for_platform(igt_display_t *display, uint32_t *mas
>> >  	}
>> >  }
>> >
>> > +/**
>> > + * igt_get_free_pipes_mask:
>> > + * @display: The display structure containing pipe information.
>> > + *
>> > + * Get the CRTCs that are currently usable and translate that into a
>> > + * bitmask of hardware pipes. A CRTC is reported free only when it is
>> > +neither
>> > + * enabled through KMS nor active in hardware, so pipes claimed by
>> > +another
>> > + * client (fbcon, a compositor, joiner) are never handed out to a modeset.
>> > + *
>> > + * The returned mask reflects the state at call time, so query it
>> > +right before
>> > + * a modeset and after releasing the pipes a previous commit was using.
>> > + *
>> > + * Returns: bitmask of hardware pipes whose CRTC is free to be
>> > +assigned. When
>> > + * libdrm lacks drmModeGetCrtcList() every pipe of @display is reported
>> free.
>> > + */
>> > +uint32_t igt_get_free_pipes_mask(igt_display_t *display) {
>> > +	uint32_t free_mask = 0;
>> > +#ifdef HAVE_DRMMODEGETCRTCLIST
>> 
>> #ifdefs belong outside of functions, not inside.
>> 
> If DrmModeGetCrtcList() is not available in the libdrm, i.e an older libdrm then the legacy method of getting crtc is used, which is handled in the #else part.

Yeah, but #ifdefs still belong outside of functions, not inside.

I'm not sure why I need to explain this in exhaustive detail.

Don't do this:

int foo(void)
{
#ifdef FOO
	return 1;
#else
	return 0;
#endif
}

Do this:

#ifdef FOO
int foo(void)
{
	return 1;
}
#else
int foo(void)
{
	return 0;
}
#endif

This is a contrived example; it makes a ton more sense when your
function is long and non-trivial, and the conditional compilation
branches are long. Like in the function you're adding. #ifdefs make it
hard to follow the code. It's much easier to read functions that don't
have conditional compilation inside them.

BR,
Jani.


>
> Thanks and Regards,
> Arun R Murthy
> --------------------
>> > +	drmModeCrtcListPtr list;
>> > +	int i;
>> > +
>> > +	list = drmModeGetCrtcList(display->drm_fd);
>> > +	igt_assert_f(list, "drmModeGetCrtcList() failed\n");
>> > +
>> > +	for (i = 0; i < list->count; i++) {
>> > +		drmModeCrtcInfoPtr info = &list->crtcs[i];
>> > +		igt_crtc_t *crtc = igt_crtc_for_crtc_id(display, info->crtc_id);
>> > +
>> > +		/* A CRTC the display never enumerated is not ours to use. */
>> > +		if (!crtc)
>> > +			continue;
>> > +
>> > +		if (info->in_use) {
>> > +			igt_debug("CRTC %s in use: %ux%u, fb %u\n",
>> > +				  igt_crtc_name(crtc), info->width,
>> > +				  info->height, info->buffer_id);
>> > +			continue;
>> > +		}
>> > +
>> > +		if (info->driver_active) {
>> > +			igt_debug("CRTC %s active in hardware, not enabled
>> via KMS\n",
>> > +				  igt_crtc_name(crtc));
>> > +			continue;
>> > +		}
>> > +
>> > +		free_mask |= BIT(crtc->hardware_pipe);
>> > +	}
>> > +
>> > +	igt_info("%d of %d CRTCs free, free pipe mask 0x%x\n",
>> > +		 list->count_available, list->count, free_mask);
>> > +
>> > +	drmModeFreeCrtcList(list);
>> > +#else
>> > +	igt_crtc_t *crtc;
>> > +
>> > +	for_each_crtc(display, crtc)
>> > +		free_mask |= BIT(crtc->hardware_pipe);
>> > +
>> > +	igt_debug("libdrm has no drmModeGetCrtcList(), assuming pipe mask
>> 0x%x free\n",
>> > +		  free_mask);
>> > +#endif
>> > +
>> > +	return free_mask;
>> > +}
>> > +
>> >  /*
>> >   * @drm_fd: DRM file descriptor
>> >   * @outputs: array of pointers to igt_output_t diff --git
>> > a/tests/intel/kms_joiner_helper.h b/tests/intel/kms_joiner_helper.h
>> > index
>> >
>> a0ab75b2c165084b643ead3f3ffc05cf5364e90b..14083d25c6f09e0577f123130
>> 697
>> > c34c2b6a15e4 100644
>> > --- a/tests/intel/kms_joiner_helper.h
>> > +++ b/tests/intel/kms_joiner_helper.h
>> > @@ -11,6 +11,7 @@
>> >
>> >  void igt_set_all_master_pipes_for_platform(igt_display_t *display,
>> >  					   uint32_t *master_pipes);
>> > +uint32_t igt_get_free_pipes_mask(igt_display_t *display);
>> >  bool igt_assign_pipes_for_outputs(int drm_fd,
>> >  				  igt_output_t **outputs,
>> >  				  int num_outputs,
>> 
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel