[igt-dev] [PATCH i-g-t 6/6] tests/kms_async_flips: Test all modifiers
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Feb 3 12:32:01 UTC 2023
On Thu, Feb 02, 2023 at 12:24:32PM +0000, Murthy, Arun R wrote:
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Sent: Thursday, February 2, 2023 5:46 PM
> > To: Murthy, Arun R <arun.r.murthy at intel.com>
> > Cc: igt-dev at lists.freedesktop.org
> > Subject: Re: [PATCH i-g-t 6/6] tests/kms_async_flips: Test all modifiers
> >
> > On Thu, Feb 02, 2023 at 11:23:09AM +0000, Murthy, Arun R wrote:
> > > > -----Original Message-----
> > > > From: Ville Syrjala <ville.syrjala at linux.intel.com>
> > > > Sent: Tuesday, January 31, 2023 5:37 PM
> > > > To: igt-dev at lists.freedesktop.org
> > > > Cc: Murthy, Arun R <arun.r.murthy at intel.com>
> > > > Subject: [PATCH i-g-t 6/6] tests/kms_async_flips: Test all modifiers
> > > >
> > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > >
> > > > Run the basic async flip test for all modifiers reported by the
> > > > plane (for the
> > > > XRGB8888 format). The driver may not support async flip with all
> > > > modifiers so we allow this to fail.
> > >
> > > Support for adding X-tile and linear for all platforms is being added
> > > as sub-test in https://patchwork.freedesktop.org/series/103137/
> > >
> > > >
> > > > TODO: probably shuould add an even more basic subtest that
> > > > makes sure the driver accepts async flips with a least
> > > > one format/modifier combo, if it advertises async flip
> > > > support
> > > >
> > > > Cc: Arun R Murthy <arun.r.murthy at intel.com>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > ---
> > > > tests/kms_async_flips.c | 54
> > > > +++++++++++++++++++++++++++++++++--------
> > > > 1 file changed, 44 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c index
> > > > 58740f6ab702..387fcff2f32c 100644
> > > > --- a/tests/kms_async_flips.c
> > > > +++ b/tests/kms_async_flips.c
> > > > @@ -53,6 +53,8 @@ typedef struct {
> > > > igt_output_t *output;
> > > > unsigned long flip_timestamp_us;
> > > > double flip_interval;
> > > > + uint64_t modifier;
> > > > + igt_plane_t *plane;
> > > > igt_pipe_crc_t *pipe_crc;
> > > > igt_crc_t ref_crc;
> > > > int flip_count;
> > > > @@ -61,6 +63,7 @@ typedef struct {
> > > > bool extended;
> > > > enum pipe pipe;
> > > > bool alternate_sync_async;
> > > > + bool allow_fail;
> > > > } data_t;
> > > >
> > > > static void flip_handler(int fd_, unsigned int sequence, unsigned
> > > > int tv_sec, @@ -133,7 +136,7 @@ static void make_fb(data_t *data,
> > struct igt_fb *fb,
> > > > rec_width = width / (ARRAY_SIZE(data->bufs) * 2);
> > > >
> > > > igt_create_color_fb(data->drm_fd, width, height,
> > > > DRM_FORMAT_XRGB8888,
> > > > - default_modifier(data), 0.0, 0.0, 0.5, fb);
> > > > + data->modifier, 0.0, 0.0, 0.5, fb);
> > > >
> > > > cr = igt_get_cairo_ctx(data->drm_fd, fb);
> > > > igt_paint_color_rand(cr, rec_width * 2 + rec_width * index, 0,
> > > > rec_width, height); @@ -159,24 +162,26 @@ static void
> > > > test_init(data_t
> > > > *data)
> > > > data->refresh_rate = mode->vrefresh;
> > > >
> > > > igt_output_set_pipe(data->output, data->pipe);
> > > > +
> > > > + data->plane = igt_output_get_plane_type(data->output,
> > > > +DRM_PLANE_TYPE_PRIMARY);
> > > > }
> > > >
> > > > static void test_init_fbs(data_t *data) {
> > > > int i;
> > > > uint32_t width, height;
> > > > - igt_plane_t *plane;
> > > > static uint32_t prev_output_id;
> > > > + static uint64_t prev_modifier;
> > > > drmModeModeInfo *mode;
> > > >
> > > > mode = igt_output_get_mode(data->output);
> > > > width = mode->hdisplay;
> > > > height = mode->vdisplay;
> > > >
> > > > - plane = igt_output_get_plane_type(data->output,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > -
> > > > - if (prev_output_id != data->output->id) {
> > > > + if (prev_output_id != data->output->id ||
> > > > + prev_modifier != data->modifier) {
> > > > prev_output_id = data->output->id;
> > > > + prev_modifier = data->modifier;
> > > >
> > > > if (data->bufs[0].fb_id) {
> > > > for (i = 0; i < ARRAY_SIZE(data->bufs); i++) @@ -187,8
> > > > +192,8 @@ static void test_init_fbs(data_t *data)
> > > > make_fb(data, &data->bufs[i], width, height, i);
> > > > }
> > > >
> > > > - igt_plane_set_fb(plane, &data->bufs[0]);
> > > > - igt_plane_set_size(plane, width, height);
> > > > + igt_plane_set_fb(data->plane, &data->bufs[0]);
> > > > + igt_plane_set_size(data->plane, width, height);
> > > >
> > > > igt_display_commit2(&data->display, data->display.is_atomic ?
> > > > COMMIT_ATOMIC : COMMIT_LEGACY); } @@ -243,8 +248,10 @@ static
> > void
> > > > test_async_flip(data_t *data)
> > > > ret = drmModePageFlip(data->drm_fd, data->crtc_id,
> > > > data->bufs[frame % 4].fb_id,
> > > > flags, data);
> > > > -
> > > > - igt_assert(ret == 0);
> > > > + if (frame == 1 && data->allow_fail)
> > > > + igt_skip_on(ret == -EINVAL);
> > > > + else
> > > > + igt_assert(ret == 0);
> > > >
> > > > wait_flip_event(data);
> > > >
> > > > @@ -545,6 +552,9 @@ static void run_test(data_t *data, void
> > > > (*test)(data_t
> > > > *))
> > > > for_each_pipe_with_valid_output(&data->display, data->pipe, data-
> > > > >output) {
> > > > test_init(data);
> > > >
> > > > + data->allow_fail = false;
> > > > + data->modifier = default_modifier(data);
> > > > +
> > > > igt_dynamic_f("pipe-%s", kmstest_pipe_name(data->pipe)) {
> > > > test_init_fbs(data);
> > > > test(data);
> > > > @@ -555,6 +565,30 @@ static void run_test(data_t *data, void
> > > > (*test)(data_t *))
> > > > }
> > > > }
> > > >
> > > > +static void run_test_with_modifiers(data_t *data, void
> > > > +(*test)(data_t
> > > > +*)) {
> > > > + for_each_pipe_with_valid_output(&data->display, data->pipe, data-
> > > > >output) {
> > > > + test_init(data);
> > > > +
> > > > + for (int i = 0; i < data->plane->format_mod_count; i++) {
> > > > + if (data->plane->formats[i] !=
> > > > DRM_FORMAT_XRGB8888)
> > > > + continue;
> > > > +
> > > > + data->allow_fail = true;
> > > If the modifier is supported by the platform and there is a failure with a
> > bug in driver/hardware, aren't we falsely reporting here?
> >
> > There is no way to query what is supported by the hw/driver, so we just have
> > to try them all and accept that not all may succeed.
> Yes that right!
> Failing for particular modifier not supported would be better than allowing to escape a failure.
It will skip, which if fine IMO.
> Moreover exceptions can be added for platforms not supporting a particular modifier.
> Maybe a proper way of handling the error might be required.
Adding i915 speicifc platform checks into the test means
we'll have to potentially touch this test for every new
platform. Which isn't a good thing.
--
Ville Syrjälä
Intel
More information about the igt-dev
mailing list