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

Ser, Simon
2019-07-12T12:54:01Z

So, to test these last two patches we'd need specific hardware right?
Because VKMS doesn't support cloning yet (does it?).

What kind of hardware supports cloned writeback outputs? I have a
Raspberry Pi which supports writeback via VC4, but I don't think it has
writeback cloning. I'm also not willing to install any proprietary
driver.

I guess we could land the first part of the series, and wait for VKMS
to support cloned outputs to land the last two patches.

Any other ideas?

On Wed, 2019-06-12 at 23:18 -0300, Brian Starkey wrote:
> An output can be added as a clone of any other output(s) attached to a
> pipe using igt_output_clone_pipe()
>
> v5: Drop field out_fence_requested from struct igt_pipe (Brian Starkey)
>
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
> lib/igt_kms.c | 100 +++++++++++++++++++++++++++++++-------------------
> lib/igt_kms.h | 4 ++
> 2 files changed, 66 insertions(+), 38 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 140db346..b85a0404 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1765,6 +1765,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;
> @@ -2317,42 +2328,6 @@ void igt_display_fini(igt_display_t *display)
> display->planes = 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;
> @@ -2376,6 +2351,40 @@ 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;
> + unsigned long pending_crtc_idx_mask;
> +
> + /* Check that outputs and pipes agree wrt. cloning */
> + for (i = 0; i < display->n_outputs; i++) {
> + output = &display->outputs[i];
> + 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,
> @@ -3766,6 +3775,7 @@ void igt_output_override_mode(igt_output_t *output, const 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
> @@ -3773,6 +3783,16 @@ void igt_output_override_mode(igt_output_t *output, const drmModeModeInfo *mode)
> }
> }
>
> +void igt_output_clone_pipe(igt_output_t *output, enum pipe pipe)
> +{
> + igt_display_t *display = output->display;
> + uint32_t current_clones = display->pipes[pipe].outputs;
> +
> + igt_output_set_pipe(output, pipe);
> +
> + display->pipes[pipe].outputs |= current_clones;
> +}
> +
> /*
> * igt_output_set_pipe:
> * @output: Target output for which the pipe is being set to
> @@ -3789,11 +3809,15 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
>
> igt_assert(output->name);
>
> - if (output->pending_pipe != PIPE_NONE)
> + if (output->pending_pipe != PIPE_NONE) {
> old_pipe = igt_output_get_driving_pipe(output);
> + old_pipe->outputs &= ~(1 << igt_output_idx(output));
> + }
>
> - if (pipe != PIPE_NONE)
> + if (pipe != PIPE_NONE) {
> pipe_obj = &display->pipes[pipe];
> + pipe_obj->outputs = (1 << igt_output_idx(output));
> + }
>
> LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
> kmstest_pipe_name(pipe));
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index cacc6b90..676839bb 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -354,6 +354,8 @@ struct igt_pipe {
> uint32_t crtc_id;
>
> int32_t out_fence_fd;
> +
> + uint32_t outputs;
> };
>
> typedef struct {
> @@ -411,6 +413,8 @@ const char *igt_output_name(igt_output_t *output);
> drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
> void igt_output_override_mode(igt_output_t *output, const drmModeModeInfo *mode);
> void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
> +void igt_output_clone_pipe(igt_output_t *output, enum pipe pipe);
> +
> igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
> igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
> int igt_output_count_plane_type(igt_output_t *output, int plane_type);
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev