Re: [PATCH i-g-t v2 2/2] tests/kms_setmode: Add basic-no-cpu-idle subtest

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

On Thu, 2026-07-09 at 16:05 +0200, Kamil Konieczny wrote:
> Hi Jason-JH,
> On 2026-06-21 at 16:43:28 +0800, Jason-JH Lin wrote:
> > Add a new subtest that disables CPU deep sleep to eliminate wakeup
> > latency interference during vblank timestamp verification.
> > 
> > This allows platforms without get_scanout_position() to verify
> > their
> > timestamp accuracy is correct, separate from CPU idle interference.
> > 
> > The original "basic" test remains unchanged to detect any timestamp
> > issues including CPU idle interference, while "basic-no-cpu-idle"
> > specifically tests timestamp accuracy without external
> > interference.
> > 
> > Once get_scanout_position() is implemented, the vblank timestamp
> > will be accurately compensated regardless of CPU idle states.
> > 
> > Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> > ---
> >  tests/kms_setmode.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> > index 3adc4534ce88..e5f2c4fee141 100644
> > --- a/tests/kms_setmode.c
> > +++ b/tests/kms_setmode.c
> > @@ -42,6 +42,7 @@
> >  #include <math.h>
> >  
> >  #include "i915/intel_drrs.h"
> > +#include "igt_pm.h"
> >  #include "xe/xe_query.h"
> >  
> >  /**
> > @@ -49,6 +50,13 @@
> >   * Description: Tests the vblank timing by iterating through all
> > valid crtc/connector
> >   *              combinations
> >   *
> > + * SUBTEST: basic-no-cpu-idle
> > + * Description: Tests vblank timing with CPU deep sleep disabled
> > to verify
> > + *              timestamp accuracy without CPU idle interference.
> > This test
> > + *              is for platforms lacking HW/SW
> > get_scanout_position() support,
> > + *              allowing them to verify vblank timestamp
> > correctness without
> > + *              external CPU wakeup latency interference.
> > + *
> >   * SUBTEST: basic-clone-single-crtc
> >   * Description: Test allows the use of a single CRTC for two
> > connectors, such as VGA/HDMI,
> >   *		VGA/DP, and HDMI/HDMI
> > @@ -1106,6 +1114,30 @@ int igt_main_args("det:", NULL, help_str,
> > opt_handler, NULL)
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * This subtest disables CPU deep sleep to eliminate
> > wakeup latency
> > +	 * interference during vblank timestamp verification.
> > +	 *
> > +	 * NOTE: This is for platforms that have not yet
> > implemented
> > +	 * get_scanout_position(). Once get_scanout_position() is
> > properly
> > +	 * implemented, the vblank timestamp will be accurately
> > compensated
> > +	 * regardless of CPU idle states, and this test becomes
> > redundant
> > +	 * with the standard "basic" test.
> > +	 */
> > +	igt_describe("Tests vblank timing with CPU deep sleep
> > disabled");
> > +	igt_subtest_with_dynamic("basic-no-cpu-idle") {
> > +		struct test_config tconf = {
> > +			.flags		= TEST_TIMINGS,
> > +			.name		= "basic-no-cpu-idle",
> > +			.resources	= drm_resources,
> > +		};
> > +
> > +		igt_require_f(igt_pm_disable_cpu_deep_sleep(),
> > +			      "Platform does not support CPU idle
> > control\n");
> > +		run_test(&tconf);
> > +		igt_pm_enable_cpu_deep_sleep();
> 
> Seems ok but this enable will be skipped in case of test fail.
> imho you should add enabling to at_exit path.
> 

Thanks for the review! I initially thought the enable call was only
needed when disable succeeded, but I missed the case where run_test()
fails midway after a successful disable.

I'll add igt_install_exit_handler() to ensure
igt_pm_enable_cpu_deep_sleep() is always called after a successful
disable, regardless of test outcome.

E,g.
        igt_require_f(igt_pm_disable_cpu_deep_sleep(),
                  "Platform does not support CPU idle control\n");
+       igt_install_exit_handler(restore_cpu_deep_sleep);
        run_test(&tconf);
-       igt_pm_enable_cpu_deep_sleep();

Regards,
Jasdon-JH Lin


> Regards,
> Kamil
> 
> > +	}
> > +
> >  	igt_fixture() {
> >  		drmModeFreeResources(drm_resources);
> >  		drm_close_driver(drm_fd);
> > -- 
> > 2.43.0
> >