[PATCH i-g-t v4] tests/intel/kms_pm_dc: Add skip logic and logging for DC5 psr test

[Kamil Konieczny]

Hi Mohammed,
On 2025-03-21 at 23:22:59 +0530, Mohammed Thasleem wrote:
> Prior to Gen12, Pipe A is required to run the DC5 transaction,
> ensuring low power operation. In display versions greater than 12,
> either Pipe A or Pipe B can be used for the DC5 transaction,
> providing flexibility in pipe selection
> 
> However, display version 20 specifically requires Pipe A.
> Additionally, for Battlemage and DG2 devices, only Pipe A is
> required due to specific hardware constraints. To enforce these
> conditions, skip logic and logging have been added, ensuring that
> tests are only executed when the appropriate pipes are selected.
> 
> v2: Updated subject title and discription.
>     Add Gen checks for low power pipe selection. (Santosh)
> v3: Add check for bmg and dg2 for Pipe A also other checks for Pipe A
>     and B. (Gustavo)
> v4: Update logic style. (Gustavo)
> 
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem at intel.com>
> Reviewed-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
> Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>
> ---
>  tests/intel/kms_pm_dc.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
> index 50473321a..4b2464c24 100644
> --- a/tests/intel/kms_pm_dc.c
> +++ b/tests/intel/kms_pm_dc.c
> @@ -145,13 +145,26 @@ static void set_output_on_pipe_b(data_t *data)
>  
>  static void setup_output(data_t *data)
>  {
> +	int disp_ver = intel_display_ver(data->devid);
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
> +	bool is_low_power;
>  	enum pipe pipe;
>  
>  	for_each_pipe_with_valid_output(display, pipe, output) {
>  		drmModeConnectorPtr c = output->config.connector;
>  
> +		if (disp_ver >= 13) {
> +			if (disp_ver == 20 || IS_BATTLEMAGE(data->devid) || IS_DG2(data->devid))
> +				is_low_power = (pipe == PIPE_A);
> +			else
> +				is_low_power = (pipe == PIPE_A || pipe == PIPE_B);
> +		} else {
> +			is_low_power = (pipe == PIPE_A);
> +		}
> +
> +		igt_skip_on_f(!is_low_power, "Low power pipe was not selected for the DC5 transaction.\n");

Why do you skip here? Imho it could debug print about low power
and continue, just like below if ... CONNECTOR_eDP, so

		if (!is_low_power) {
			igt_debug("Low power pipe%c was not selected for the DC5 transaction.\n",
				   pipe == PIPE_A ? 'A' : 'B');
			continue;
		}

Btw I hope that there is later on a check that anything was
actually tested.

Regards,
Kamil

> +
>  		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
>  			continue;
>  
> -- 
> 2.43.0
>