[PATCH i-g-t v4 3/4] lib/igt_kms: loosen duplicate check in igt_display_refresh

[Abhinav Kumar]

On 9/25/2024 1:37 PM, Jessica Zhang wrote:
> From: Esha Bharadwaj <quic_ebharadw at quicinc.com>
> 
> Change the duplicate check in igt_display_refresh() so it allows for
> pipes to be shared by encoders that are valid clones of each other.
> 
> Signed-off-by: Esha Bharadwaj <quic_ebharadw at quicinc.com>
> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
> ---
>   lib/igt_kms.c | 42 +++++++++++++++++++++++++++++++++---------
>   lib/igt_kms.h |  1 +
>   2 files changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f5fab7f73..a17efc364 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3286,22 +3286,46 @@ int kmstest_get_encoder_idx(drmModeRes *resources, drmModeEncoder *encoder)
>   	igt_assert(0);
>   }
>   
> -static void igt_display_refresh(igt_display_t *display)
> +bool igt_output_clone_valid(int drm_fd, igt_output_t *target, igt_output_t *clone)
>   {
> -	igt_output_t *output;
> -	int i;
> +	drmModeEncoder *target_enc;
> +	drmModeEncoder *clone_enc;
> +	drmModeRes *resources = drmModeGetResources(drm_fd);
> +	uint32_t clone_mask;
> +
> +	if (!target || !clone)
> +		return false;
> +
> +	target_enc = target->config.encoder;
> +	clone_enc = clone->config.encoder;
> +
> +	if (!target_enc || !clone_enc)
> +		return false;
> +
> +	clone_mask = 1 << kmstest_get_encoder_idx(resources, clone_enc);
>   
> -	unsigned long pipes_in_use = 0;
> +	return ((target_enc->possible_clones & clone_mask) != clone_mask);
> +}
> +
> +static void igt_display_refresh(igt_display_t *display)
> +{
> +	igt_output_t *output, *clone_output;
> +	int i, j;
>   
> -       /* Check that two outputs aren't trying to use the same pipe */
>   	for (i = 0; i < display->n_outputs; i++) {
>   		output = &display->outputs[i];
>   
> -		if (output->pending_pipe != PIPE_NONE) {
> -			if (pipes_in_use & (1 << output->pending_pipe))
> -				goto report_dup;
> +		for (j = i + 1; j < display->n_outputs; j++) {
> +			clone_output = &display->outputs[j];
>   
> -			pipes_in_use |= 1 << output->pending_pipe;

We should not be dropping pipes_in_use.

I do agree that this check is perhaps a bit outdated that it does not 
consider clone mode and we should account for it before deeming a 
duplicate but the check should actually be

if (pipes_in_use & (1 << output->pending_pipe)) &&
	!igt_output_clone_valid())
	goto report_dup;

Otherwise this will break the existing API which catches the non-clone 
mode case.

> +			/*
> +			 * Check that any encoders with duplicated pipes are
> +			 * possible clones of each other. If they aren't, report
> +			 * the pipe as duplicated
> +			 */
> +			if (!igt_output_clone_valid(display->drm_fd,
> +						output, clone_output))
> +				goto report_dup;
>   		}
>