[igt-dev] [PATCH i-g-t v4 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning

[Maarten Lankhorst]

Op 05-11-18 om 11:16 schreef Liviu Dudau:
> From: Brian Starkey <brian.starkey at arm.com>
>
> An output can be added as a clone of any other output(s) attached to a
> pipe using igt_output_clone_pipe()
>
> Signed-off-by: Brian Starkey <brian.starkey at arm.com>
> ---
>  lib/igt_kms.c | 99 +++++++++++++++++++++++++++++++--------------------
>  lib/igt_kms.h |  5 +++
>  2 files changed, 66 insertions(+), 38 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 00d356fe0..0a826718e 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1694,6 +1694,17 @@ static void igt_display_log_shift(igt_display_t *display, int shift)
>  	igt_assert(display->log_shift >= 0);
>  }
>  
> +static int igt_output_idx(igt_output_t *output)
> +{
> +	int i;
> +
> +	for (i = 0; i < output->display->n_outputs; i++)
> +		if (&output->display->outputs[i] == output)
> +			return i;
> +
> +	return -1;
> +}
> +
>  static void igt_output_refresh(igt_output_t *output)
>  {
>  	igt_display_t *display = output->display;
> @@ -2188,42 +2199,6 @@ void igt_display_fini(igt_display_t *display)
>  	display->pipes = NULL;
>  }
>  
> -static void igt_display_refresh(igt_display_t *display)
> -{
> -	igt_output_t *output;
> -	int i;
> -
> -	unsigned long pipes_in_use = 0;
> -
> -       /* 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;
> -
> -			pipes_in_use |= 1 << output->pending_pipe;
> -		}
> -
> -		if (output->force_reprobe)
> -			igt_output_refresh(output);
> -	}
> -
> -	return;
> -
> -report_dup:
> -	for (; i > 0; i--) {
> -		igt_output_t *b = &display->outputs[i - 1];
> -
> -		igt_assert_f(output->pending_pipe !=
> -			     b->pending_pipe,
> -			     "%s and %s are both trying to use pipe %s\n",
> -			     igt_output_name(output), igt_output_name(b),
> -			     kmstest_pipe_name(output->pending_pipe));
> -	}
> -}
> -
>  static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
>  {
>  	igt_display_t *display = output->display;
> @@ -2247,6 +2222,39 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
>  	return &display->pipes[pipe];
>  }
>  
> +static void igt_display_refresh(igt_display_t *display)
> +{
> +	igt_output_t *output;
> +	igt_pipe_t *pipe;
> +	int i;
> +
> +	unsigned long pipes_in_use = 0;
> +
> +	/* Check that outputs and pipes agree wrt. cloning */
> +	for (i = 0; i < display->n_outputs; i++) {
> +		output = &display->outputs[i];
> +		unsigned long pending_crtc_idx_mask = 1 << output->pending_pipe;
> +
> +		pipe = igt_output_get_driving_pipe(output);
> +		if (pipe) {
> +			igt_assert_f(pipe->outputs & (1 << igt_output_idx(output)),
> +				     "Output %s not expected to be using pipe %s\n",
> +				     igt_output_name(output),
> +				     kmstest_pipe_name(pipe->pipe));
> +
> +			if (pipes_in_use & pending_crtc_idx_mask)
> +				LOG(display, "Output %s clones pipe %s\n",
> +				    igt_output_name(output),
> +				    kmstest_pipe_name(pipe->pipe));
> +		}
> +
> +		pipes_in_use |= pending_crtc_idx_mask;
> +
> +		if (output->force_reprobe)
> +			igt_output_refresh(output);
> +	}
> +}
> +
>  static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int plane_idx)
>  {
>  	igt_require_f(plane_idx >= 0 && plane_idx < pipe->n_planes,
> @@ -3583,6 +3591,7 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
>  	output->use_override_mode = !!mode;
>  
>  	if (pipe) {
> +		igt_debug("overriding pipe mode in %s way\n", output->display->is_atomic ? "atomic" : "legacy");
>  		if (output->display->is_atomic)
>  			igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
>  		else

I would move the mode being set to igt_pipe, kill off the internal
igt_pipe_get_output function; replace igt_output_override_mode()
with igt_pipe(,_obj)_override_mode functions, and fix
igt_primary_commit_legacy to use more than 1 output.

These are all the places why we assume only 1 pipe exists.

Looks also like igt_pipe_refresh might be replaced with igt_display_reset,
or at least igt_pipe_obj_set_prop_changed must be aclled unconditionally,
but that's not necessarily a big change.

~Maarten