[igt-dev] [PATCH i-g-t] tests/kms_cursor_crc: Fix user space read too slow error

[Chris Wilson]

Quoting Juha-Pekka Heikkilä (2019-11-29 19:57:48)
> On Fri, Nov 29, 2019 at 6:04 PM Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Quoting Juha-Pekka Heikkila (2019-11-29 15:52:45)
> > > Having crc running continuously cause this test sometime
> > > fill crc buffer, fix this problem as well as do some generic
> > > cleanups.
> > >
> > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> > > ---
> > >  tests/kms_cursor_crc.c | 106 +++++++++++++++++++++++++------------------------
> > >  1 file changed, 54 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > > index 6c4c457..6982437 100644
> > > --- a/tests/kms_cursor_crc.c
> > > +++ b/tests/kms_cursor_crc.c
> > > @@ -52,7 +52,6 @@ typedef struct {
> > >         struct igt_fb fb;
> > >         igt_output_t *output;
> > >         enum pipe pipe;
> > > -       igt_crc_t ref_crc;
> > >         int left, right, top, bottom;
> > >         int screenw, screenh;
> > >         int refresh;
> > > @@ -60,6 +59,9 @@ typedef struct {
> > >         int cursor_max_w, cursor_max_h;
> > >         igt_pipe_crc_t *pipe_crc;
> > >         unsigned flags;
> > > +       igt_plane_t *primary;
> > > +       igt_plane_t *cursor;
> > > +       cairo_surface_t *surface;
> > >  } data_t;
> > >
> > >  #define TEST_DPMS (1<<0)
> > > @@ -89,23 +91,15 @@ static void draw_cursor(cairo_t *cr, int x, int y, int cw, int ch, double a)
> > >
> > >  static void cursor_enable(data_t *data)
> > >  {
> > > -       igt_output_t *output = data->output;
> > > -       igt_plane_t *cursor =
> > > -               igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR);
> > > -
> > > -       igt_plane_set_fb(cursor, &data->fb);
> > > -       igt_plane_set_size(cursor, data->curw, data->curh);
> > > -       igt_fb_set_size(&data->fb, cursor, data->curw, data->curh);
> > > +       igt_plane_set_fb(data->cursor, &data->fb);
> > > +       igt_plane_set_size(data->cursor, data->curw, data->curh);
> > > +       igt_fb_set_size(&data->fb, data->cursor, data->curw, data->curh);
> > >  }
> > >
> > >  static void cursor_disable(data_t *data)
> > >  {
> > > -       igt_output_t *output = data->output;
> > > -       igt_plane_t *cursor =
> > > -               igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR);
> > > -
> > > -       igt_plane_set_fb(cursor, NULL);
> > > -       igt_plane_set_position(cursor, 0, 0);
> > > +       igt_plane_set_fb(data->cursor, NULL);
> > > +       igt_plane_set_position(data->cursor, 0, 0);
> > >  }
> > >
> > >  static bool chv_cursor_broken(data_t *data, int x)
> > > @@ -144,30 +138,40 @@ static bool cursor_visible(data_t *data, int x, int y)
> > >         return true;
> > >  }
> > >
> > > +static void restore_image(data_t *data)
> > > +{
> > > +       cairo_t *cr;
> > > +
> > > +       cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> > > +       cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> > > +       cairo_set_source_surface(cr, data->surface, 0, 0);
> > > +       cairo_rectangle(cr, 0, 0, data->screenw, data->screenh);
> > > +       cairo_fill(cr);
> > > +       igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> > > +       gem_sync(data->drm_fd, data->primary_fb.gem_handle);
> >
> > This is a generic KMS test, gem_sync() is not supported.
> 
> Idea here was to make certain memory copied to FB is really there. I
> did ask from Mika for ideas and he suggested changing domain to cause
> synchronization, in ioctl_wrappers.c at gem_set_domain() comments
> suggested to use gem_sync() instead.

Then you are trying to paper over real bugs. Userspace calls dirtyfb to
declare it has updated the fb, the kernel then has to take care of
making sure it is coherent prior to the next flip, or worry more if it
is already on the scanout.
-Chris