On 07/12, Ser, Simon wrote:
> So, to test these last two patches we'd need specific hardware right?
> Because VKMS doesn't support cloning yet (does it?).

hmmm... actually, VKMS successfully pass in this test. However, if you
compare "writeback-check-output" and "writeback-check-output-clone", you
will notice they are very similar. Maybe, this test does not correctly
validating cloning feature?

> 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?

btw, I'm totally comfortable with the idea of focusing on the first part
of this series.

Thanks
 
> 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.star...@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-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Rodrigo Siqueira
https://siqueira.tech

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to