Re: [PATCH i-g-t v4 3/7] tests/intel/kms_pm_dc: Enable DC3CO test for PSR2/PR modes

["Naladala, Ramanaidu" <Ramanaidu.naladala@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4555 bytes --]

Hi Jeevan,

On 5/11/2026 10:48 PM, Jeevan B wrote:
> Enable DC3CO with PSR2/PR mode on TGL and for platforms with
> display version greater than 35.

This test is also applicable to display version 35, so the commit 
message should be updated accordingly to reflect that support accurately.

like, Enable DC3CO with PSR2/PR mode on TGL and for platforms with 
display version 35 or newer.

>
> v2: Fix debug, remove trailing dash and merge mode and char to
>      single strcut array.
> v3: Minor cosmetic changes.
>
> Signed-off-by: Jeevan B<jeevan.b@intel.com>
> Reviewed-by: Mohammed Thasleem<mohammed.thasleem@intel.com>
> ---
>   tests/intel/kms_pm_dc.c | 50 ++++++++++++++++++++++++++++++++---------
>   1 file changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
> index 27fa5dc39..83652e9f8 100644
> --- a/tests/intel/kms_pm_dc.c
> +++ b/tests/intel/kms_pm_dc.c
> @@ -110,6 +110,11 @@ typedef struct {
>   	bool runtime_suspend_disabled;
>   } data_t;
>   
> +struct dc3co_test_mode {
> +	enum psr_mode mode;
> +	const char *name;
> +};
> +
>   static void assert_dc_counter(data_t *data, int dc_flag, uint32_t prev_dc_count);
>   
>   static void set_output_on_pipe_b(data_t *data)
> @@ -319,18 +324,20 @@ static void check_dc3co_with_videoplayback_like_load(data_t *data)
>   	assert_dc_counter(data, IGT_INTEL_CHECK_DC3CO, dc3co_prev_cnt);
>   }
>   
> -static void setup_dc3co(data_t *data)
> +static void setup_dc3co(data_t *data, enum psr_mode mode)

This function doesn’t introduce any DC3CO-related updates. It only 
adjusts PSR enable handling. Given that, renaming the function from 
|setup_dc3co()| to |setup_psr()| would better reflect its actual purpose.

Also, since the required PSR mode is already stored earlier during the 
dynamic subtest, passing |enum psr_mode mode| to this function isn’t 
necessary. The function can directly use |data->op_psr_mode|.

So the final function can be: static void setup_dc3co(data_t *data)

>   {
> +	data->op_psr_mode = mode;
>   	psr_enable(data->drm_fd, data->debugfs_fd, data->op_psr_mode, data->output);
> -	igt_require_f(psr_wait_entry(data->debugfs_fd, data->op_psr_mode, data->output),
> -		      "PSR2 is not enabled\n");
> +	igt_require_f(psr_wait_entry(data->debugfs_fd, data->op_psr_mode, NULL),
Instead of passing |NULL| to |psr_wait_entry()|, it would be better to 
continue using |data->output to | avoid losing output-specific context 
during the PSR entry check.
> +		      "%s is not enabled\n",
> +		      mode == PSR_MODE_2 ? "PSR2" : "Panel Replay");
>   }
>   
> -static void test_dc3co_vpb_simulation(data_t *data)
> +static void test_dc3co_vpb_simulation(data_t *data, enum psr_mode mode)
>   {
>   	igt_require_dc_counter(data->debugfs_fd, IGT_INTEL_CHECK_DC3CO);
>   	setup_output(data);
> -	setup_dc3co(data);
> +	setup_dc3co(data, mode);
>   	setup_videoplayback(data);
>   	check_dc3co_with_videoplayback_like_load(data);
>   	cleanup_dc3co_fbs(data);
> @@ -658,12 +665,33 @@ int igt_main()
>   	}
>   
>   	igt_describe("In this test we make sure that system enters DC3CO "
> -		     "when PSR2 is active and system is in SLEEP state");
> -	igt_subtest("dc3co-vpb-simulation") {
> -		data.op_psr_mode = PSR_MODE_2;
> -		igt_require(psr_sink_support(data.drm_fd, data.debugfs_fd,
> -					     data.op_psr_mode, NULL));
> -		test_dc3co_vpb_simulation(&data);
> +			"when PSR2 or PR is active and system is in SLEEP state");
> +	igt_subtest_with_dynamic("dc3co-vpb-simulation") {
> +		static const struct dc3co_test_mode dc3co_modes[] = {
> +			{ PSR_MODE_2, "psr2" },
> +			{ PR_MODE,    "pr"   },
> +		};
> +
> +		for (int i = 0; i < ARRAY_SIZE(dc3co_modes); i++) {
> +			enum psr_mode mode = dc3co_modes[i].mode;
A new enum isn’t required here. The PSR mode can be stored directly in 
|data->op_psr_mode.|
> +			const char *name = dc3co_modes[i].name;
> +
> +			igt_dynamic_f("%s", name) {
> +				igt_require(psr_sink_support(data.drm_fd,
> +							     data.debugfs_fd,
> +							     mode, NULL));
> +
> +				if (mode == PSR_MODE_2)
> +					igt_require_f(IS_TIGERLAKE(data.devid) ||
> +						      intel_display_ver(data.devid) >= 35,
> +						      "Platform does not support DC3CO with PSR2\n");
> +				else
> +					igt_require_f(intel_display_ver(data.devid) >= 35,
> +						      "Platform does not support DC3CO with Panel Replay\n");
> +
> +				test_dc3co_vpb_simulation(&data, mode);
> +			}
> +		}
>   	}
>   
>   	igt_describe("This test validates display engine entry to DC5 state "
Thanks,

[-- Attachment #2: Type: text/html, Size: 7547 bytes --]