Re: [PATCH i-g-t] tests/intel/kms_pm_backlight: Setup output before reading writing backlight

[Karthik B S <karthik.b.s@intel.com>]

Hi Suraj,

On 3/31/2026 1:04 PM, Suraj Kandpal wrote:
> Currently many pm backlight tests assume that panel will stay up until
> unless we tell it to go down or AUX transactions are happening.
> That is not the case, some panels when they detect that there are no
> changes or commits going in, it will go to a lower power state. This
> causes issue in this test like AUX suddenly timing out (mostly in
> fade tests where each backlight change has a 10ms sleep and many
> steps giving panel to internally go in low power state). Also since
> changing backlight has its own interface it does not count as a commit
> hence does not contribute to the panel staying on.
> Also trying to change a backlight when panel is not up does not make sense.
> To solve this setup the output and do a flip to make sure panel is not
> down to simulate a real life use case on when backlight is changed.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   tests/intel/kms_pm_backlight.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
>
> diff --git a/tests/intel/kms_pm_backlight.c b/tests/intel/kms_pm_backlight.c
> index daa70c716..1e1e0fdbd 100644
> --- a/tests/intel/kms_pm_backlight.c
> +++ b/tests/intel/kms_pm_backlight.c
> @@ -84,6 +84,24 @@ enum {
>   
>   IGT_TEST_DESCRIPTION("Basic backlight sysfs test");
>   
> +static void setup_output_and_commit(igt_backlight_context_t *context)
> +{
> +	igt_output_t *output = context->output;
> +	igt_display_t *display = output->display;
> +	drmModeModeInfo *mode;
> +	igt_plane_t *primary;
> +	struct igt_fb fb;
> +
> +	mode = igt_output_get_mode(output);
> +	igt_create_pattern_fb(display->drm_fd,
> +			      mode->hdisplay, mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      DRM_FORMAT_MOD_LINEAR, &fb);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, &fb);
> +	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> +}
> +
>   static void test_and_verify(igt_backlight_context_t *context, int val)
>   {
>   	const int tolerance = val * TOLERANCE / 100;
> @@ -133,10 +151,12 @@ static void test_fade(igt_backlight_context_t *context)
>   
>   	/* Fade out, then in */
>   	for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
> +		setup_output_and_commit(context);
>   		test_and_verify(context, i);
>   		nanosleep(&ts, NULL);
>   	}
>   	for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
> +		setup_output_and_commit(context);
>   		test_and_verify(context, i);
>   		nanosleep(&ts, NULL);
>   	}
> @@ -154,6 +174,7 @@ static void check_dpms_cycle(igt_backlight_context_t *context)
>   
>   	igt_pm_dpms_toggle(context->output);
>   
> +	setup_output_and_commit(context);

This is not required for the dpms subtest as we already ensure we're in 
'IGT_RUNTIME_PM_STATUS_ACTIVE'. With this removed,

Reviewed-by: Karthik B S <karthik.b.s@intel.com>

>   	igt_backlight_read(&val_2, "actual_brightness", context);
>   	igt_assert_eq(val_1, val_2);
>   }