Re: [igt-dev] [PATCH i-g-t v6 without-hack-patches 1/5] tests/kms_frontbuffer_tracking: Add tiling to test_mode

Souza, Jose
2020-04-07T16:29:51Z

On Mon, 2020-04-06 at 17:26 +0300, Juha-Pekka Heikkila wrote:
> I wasn't big fan of new introduced enum but it seems to be in line
> with
> style of the test.
>
> patches 1, 2 and 3 are
> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

Thanks, pushed the 3 patches.
Will resend the 2 remaning.

>
> On 25.3.2020 21.18, José Roberto de Souza wrote:
> > This will allow us to do tests with different tile types, for now
> > all tests will continue to run with the default X tiling.
> > It will be used in upcoming patches.
> >
> > It also allow user to run all tests with liner tiling when set by
> > parameter.
> >
> > v2:
> > - renamed tile to tiling (Ville)
> > - added to the commit message that this will be used in upcoming
> > patches
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > tests/kms_frontbuffer_tracking.c | 122 ++++++++++++++++++++----
> > -------
> > 1 file changed, 78 insertions(+), 44 deletions(-)
> >
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index c4b4af43..c4deb72b 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -130,6 +130,14 @@ struct test_mode {
> > FLIP_COUNT,
> > } flip;
> >
> > + enum tiling_type {
> > + TILING_LINEAR = 0,
> > + TILING_X,
> > + TILING_Y,
> > + TILING_COUNT,
> > + TILING_DEFAULT = TILING_X,
> > + } tiling;
> > +
> > enum igt_draw_method method;
> > };
> >
> > @@ -235,7 +243,7 @@ struct {
> > int only_pipes;
> > int shared_fb_x_offset;
> > int shared_fb_y_offset;
> > - uint64_t tiling;
> > + enum tiling_type tiling;
> > } opt = {
> > .check_status = true,
> > .check_crc = true,
> > @@ -248,7 +256,7 @@ struct {
> > .only_pipes = PIPE_COUNT,
> > .shared_fb_x_offset = 248,
> > .shared_fb_y_offset = 500,
> > - .tiling = LOCAL_I915_FORMAT_MOD_X_TILED,
> > + .tiling = TILING_DEFAULT,
> > };
> >
> > struct modeset_params {
> > @@ -444,13 +452,26 @@ static bool init_modeset_cached_params(void)
> > return true;
> > }
> >
> > +static uint64_t tiling_to_modifier(enum tiling_type tiling)
> > +{
> > + switch (tiling) {
> > + case TILING_LINEAR:
> > + return LOCAL_DRM_FORMAT_MOD_NONE;
> > + case TILING_X:
> > + return LOCAL_I915_FORMAT_MOD_X_TILED;
> > + case TILING_Y:
> > + return LOCAL_I915_FORMAT_MOD_Y_TILED;
> > + default:
> > + igt_assert(false);
> > + }
> > +}
> > +
> > static void create_fb(enum pixel_format pformat, int width, int
> > height,
> > - uint64_t tiling, int plane, struct igt_fb *fb)
> > + enum tiling_type tiling, int plane, struct igt_fb
> > *fb)
> > {
> > uint32_t format;
> > - uint64_t size;
> > + uint64_t size, modifier;
> > unsigned int stride;
> > - uint64_t tiling_for_size;
> >
> > switch (pformat) {
> > case FORMAT_RGB888:
> > @@ -480,19 +501,14 @@ static void create_fb(enum pixel_format
> > pformat, int width, int height,
> > igt_assert(false);
> > }
> >
> > - /* We want all frontbuffers with the same width/height/format
> > to have
> > - * the same size regardless of tiling since we want to properly
> > exercise
> > - * the Kernel's specific tiling-checking code paths without
> > accidentally
> > - * hitting size-checking ones first. */
> > - if (plane == PLANE_CUR)
> > - tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
> > - else
> > - tiling_for_size = opt.tiling;
> > + modifier = tiling_to_modifier(tiling);
> >
> > - igt_calc_fb_size(drm.fd, width, height, format,
> > tiling_for_size, &size,
> > + igt_warn_on(plane == PLANE_CUR && tiling != TILING_LINEAR);
> > +
> > + igt_calc_fb_size(drm.fd, width, height, format, modifier,
> > &size,
> > &stride);
> >
> > - igt_create_fb_with_bo_size(drm.fd, width, height, format,
> > tiling,
> > + igt_create_fb_with_bo_size(drm.fd, width, height, format,
> > modifier,
> > IGT_COLOR_YCBCR_BT709,
> > IGT_COLOR_YCBCR_LIMITED_RANGE,
> > fb, size, stride);
> > @@ -594,7 +610,7 @@ static void fill_fb(struct igt_fb *fb, enum
> > color ecolor)
> > * We do it vertically instead of the more common horizontal case
> > in order to
> > * avoid super huge strides not supported by FBC.
> > */
> > -static void create_shared_fb(enum pixel_format format)
> > +static void create_shared_fb(enum pixel_format format, enum
> > tiling_type tiling)
> > {
> > int prim_w, prim_h, scnd_w, scnd_h, offs_w, offs_h, big_w,
> > big_h;
> > struct screen_fbs *s = &fbs[format];
> > @@ -621,7 +637,7 @@ static void create_shared_fb(enum pixel_format
> > format)
> >
> > big_h = prim_h + scnd_h + offs_h + opt.shared_fb_y_offset;
> >
> > - create_fb(format, big_w, big_h, opt.tiling, PLANE_PRI, &s-
> > >big);
> > + create_fb(format, big_w, big_h, tiling, PLANE_PRI, &s->big);
> > }
> >
> > static void destroy_fbs(enum pixel_format format)
> > @@ -643,7 +659,7 @@ static void destroy_fbs(enum pixel_format
> > format)
> > igt_remove_fb(drm.fd, &s->big);
> > }
> >
> > -static void create_fbs(enum pixel_format format)
> > +static void create_fbs(enum pixel_format format, enum tiling_type
> > tiling)
> > {
> > struct screen_fbs *s = &fbs[format];
> >
> > @@ -653,30 +669,29 @@ static void create_fbs(enum pixel_format
> > format)
> > s->initialized = true;
> >
> > create_fb(format, prim_mode_params.mode->hdisplay,
> > - prim_mode_params.mode->vdisplay, opt.tiling,
> > PLANE_PRI,
> > + prim_mode_params.mode->vdisplay, tiling, PLANE_PRI,
> > &s->prim_pri);
> > create_fb(format, prim_mode_params.cursor.w,
> > prim_mode_params.cursor.h, LOCAL_DRM_FORMAT_MOD_NONE,
> > PLANE_CUR, &s->prim_cur);
> > create_fb(format, prim_mode_params.sprite.w,
> > - prim_mode_params.sprite.h, opt.tiling, PLANE_SPR,
> > - &s->prim_spr);
> > + prim_mode_params.sprite.h, tiling, PLANE_SPR, &s-
> > >prim_spr);
> >
> > - create_fb(format, offscreen_fb.w, offscreen_fb.h, opt.tiling,
> > PLANE_PRI,
> > + create_fb(format, offscreen_fb.w, offscreen_fb.h, tiling,
> > PLANE_PRI,
> > &s->offscreen);
> >
> > - create_shared_fb(format);
> > + create_shared_fb(format, tiling);
> >
> > if (!scnd_mode_params.output)
> > return;
> >
> > create_fb(format, scnd_mode_params.mode->hdisplay,
> > - scnd_mode_params.mode->vdisplay, opt.tiling,
> > PLANE_PRI,
> > + scnd_mode_params.mode->vdisplay, tiling, PLANE_PRI,
> > &s->scnd_pri);
> > create_fb(format, scnd_mode_params.cursor.w,
> > scnd_mode_params.cursor.h,
> > LOCAL_DRM_FORMAT_MOD_NONE, PLANE_CUR, &s->scnd_cur);
> > create_fb(format, scnd_mode_params.sprite.w,
> > scnd_mode_params.sprite.h,
> > - opt.tiling, PLANE_SPR, &s->scnd_spr);
> > + tiling, PLANE_SPR, &s->scnd_spr);
> > }
> >
> > static void __set_prim_plane_for_params(struct modeset_params
> > *params)
> > @@ -1177,7 +1192,7 @@ static void collect_crc(igt_crc_t *crc)
> > igt_pipe_crc_collect_crc(pipe_crc, crc);
> > }
> >
> > -static void init_blue_crc(enum pixel_format format)
> > +static void init_blue_crc(enum pixel_format format, enum
> > tiling_type tiling)
> > {
> > struct igt_fb blue;
> >
> > @@ -1185,7 +1200,7 @@ static void init_blue_crc(enum pixel_format
> > format)
> > return;
> >
> > create_fb(format, prim_mode_params.mode->hdisplay,
> > - prim_mode_params.mode->vdisplay, opt.tiling,
> > PLANE_PRI,
> > + prim_mode_params.mode->vdisplay, tiling, PLANE_PRI,
> > &blue);
> >
> > fill_fb(&blue, COLOR_PRIM_BG);
> > @@ -1211,7 +1226,7 @@ static void init_blue_crc(enum pixel_format
> > format)
> > blue_crcs[format].initialized = true;
> > }
> >
> > -static void init_crcs(enum pixel_format format,
> > +static void init_crcs(enum pixel_format format, enum tiling_type
> > tiling,
> > struct draw_pattern_info *pattern)
> > {
> > int r, r_;
> > @@ -1225,7 +1240,7 @@ static void init_crcs(enum pixel_format
> > format,
> >
> > for (r = 0; r < pattern->n_rects; r++)
> > create_fb(format, prim_mode_params.mode->hdisplay,
> > - prim_mode_params.mode->vdisplay, opt.tiling,
> > + prim_mode_params.mode->vdisplay, tiling,
> > PLANE_PRI, &tmp_fbs[r]);
> >
> > for (r = 0; r < pattern->n_rects; r++)
> > @@ -1290,7 +1305,7 @@ static void setup_modeset(void)
> > offscreen_fb.fb = NULL;
> > offscreen_fb.w = 1024;
> > offscreen_fb.h = 1024;
> > - create_fbs(FORMAT_DEFAULT);
> > + create_fbs(FORMAT_DEFAULT, opt.tiling);
> > }
> >
> > static void teardown_modeset(void)
> > @@ -1751,7 +1766,7 @@ static void set_crtc_fbs(const struct
> > test_mode *t)
> > {
> > struct screen_fbs *s = &fbs[t->format];
> >
> > - create_fbs(t->format);
> > + create_fbs(t->format, t->tiling);
> >
> > switch (t->fbs) {
> > case FBS_INDIVIDUAL:
> > @@ -1811,9 +1826,9 @@ static void prepare_subtest_data(const struct
> > test_mode *t,
> > if (need_modeset)
> > igt_display_commit(&drm.display);
> >
> > - init_blue_crc(t->format);
> > + init_blue_crc(t->format, t->tiling);
> > if (pattern)
> > - init_crcs(t->format, pattern);
> > + init_crcs(t->format, t->tiling, pattern);
> >
> > need_modeset = enable_features_for_test(t);
> > if (need_modeset)
> > @@ -2304,7 +2319,7 @@ static void flip_subtest(const struct
> > test_mode *t)
> > prepare_subtest(t, pattern);
> >
> > create_fb(t->format, params->primary.fb->width, params-
> > >primary.fb->height,
> > - opt.tiling, t->plane, &fb2);
> > + t->tiling, t->plane, &fb2);
> > fill_fb(&fb2, bg_color);
> > orig_fb = params->primary.fb;
> >
> > @@ -2353,7 +2368,7 @@ static void fliptrack_subtest(const struct
> > test_mode *t, enum flip_type type)
> > prepare_subtest(t, pattern);
> >
> > create_fb(t->format, params->primary.fb->width, params-
> > >primary.fb->height,
> > - opt.tiling, t->plane, &fb2);
> > + t->tiling, t->plane, &fb2);
> > fill_fb(&fb2, COLOR_PRIM_BG);
> > orig_fb = params->primary.fb;
> >
> > @@ -2511,7 +2526,7 @@ static void fullscreen_plane_subtest(const
> > struct test_mode *t)
> > prepare_subtest(t, pattern);
> >
> > rect = pattern->get_rect(&params->primary, 0);
> > - create_fb(t->format, rect.w, rect.h, opt.tiling, t->plane,
> > + create_fb(t->format, rect.w, rect.h, t->tiling, t->plane,
> > &fullscreen_fb);
> > /* Call pick_color() again since PRI and SPR may not support
> > the same
> > * pixel formats. */
> > @@ -2584,7 +2599,7 @@ static void scaledprimary_subtest(const
> > struct test_mode *t)
> > old_fb = reg->fb;
> >
> > create_fb(t->format, reg->fb->width, reg->fb->height,
> > - opt.tiling, t->plane, &new_fb);
> > + t->tiling, t->plane, &new_fb);
> > fill_fb(&new_fb, COLOR_BLUE);
> >
> > igt_draw_rect_fb(drm.fd, drm.bufmgr, NULL, &new_fb, t->method,
> > @@ -2679,7 +2694,7 @@ static void modesetfrombusy_subtest(const
> > struct test_mode *t)
> > prepare_subtest(t, NULL);
> >
> > create_fb(t->format, params->primary.fb->width, params-
> > >primary.fb->height,
> > - opt.tiling, t->plane, &fb2);
> > + t->tiling, t->plane, &fb2);
> > fill_fb(&fb2, COLOR_PRIM_BG);
> >
> > start_busy_thread(params->primary.fb);
> > @@ -2782,7 +2797,7 @@ static void farfromfence_subtest(const struct
> > test_mode *t)
> > prepare_subtest(t, pattern);
> > target = pick_target(t, params);
> >
> > - create_fb(t->format, params->mode->hdisplay, max_height,
> > opt.tiling,
> > + create_fb(t->format, params->mode->hdisplay, max_height, t-
> > >tiling,
> > t->plane, &tall_fb);
> >
> > fill_fb(&tall_fb, COLOR_PRIM_BG);
> > @@ -2859,7 +2874,7 @@ static void badstride_subtest(const struct
> > test_mode *t)
> > old_fb = params->primary.fb;
> >
> > create_fb(t->format, params->primary.fb->width + 4096, params-
> > >primary.fb->height,
> > - opt.tiling, t->plane, &wide_fb);
> > + t->tiling, t->plane, &wide_fb);
> > igt_assert(wide_fb.strides[0] > 16384);
> >
> > fill_fb(&wide_fb, COLOR_PRIM_BG);
> > @@ -3039,7 +3054,7 @@ static void basic_subtest(const struct
> > test_mode *t)
> > prepare_subtest(t, pattern);
> >
> > create_fb(t->format, params->primary.fb->width, params-
> > >primary.fb->height,
> > - opt.tiling, t->plane, &fb2);
> > + t->tiling, t->plane, &fb2);
> > fb1 = params->primary.fb;
> >
> > for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT;
> > method++) {
> > @@ -3120,10 +3135,12 @@ static int opt_handler(int option, int
> > option_index, void *data)
> > break;
> > case 'l':
> > if (!strcmp(optarg, "x"))
> > - opt.tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> > + opt.tiling = TILING_X;
> > else if (!strcmp(optarg, "y"))
> > - opt.tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> > - else {
> > + opt.tiling = TILING_Y;
> > + else if (!strcmp(optarg, "l")) {
> > + opt.tiling = TILING_LINEAR;
> > + } else {
> > igt_warn("Bad tiling value: %s\n", optarg);
> > return IGT_OPT_HANDLER_ERROR;
> > }
> > @@ -3255,9 +3272,24 @@ static const char *flip_str(enum flip_type
> > flip)
> > }
> > }
> >
> > +static const char *tiling_str(enum tiling_type tiling)
> > +{
> > + switch (tiling) {
> > + case TILING_LINEAR:
> > + return "linear";
> > + case TILING_X:
> > + return "x";
> > + case TILING_Y:
> > + return "y";
> > + default:
> > + igt_assert(false);
> > + }
> > +}
> > +
> > #define TEST_MODE_ITER_BEGIN(t) \
> > t.format = FORMAT_DEFAULT;
> > \
> > t.flip = FLIP_PAGEFLIP;
> > \
> > + t.tiling = opt.tiling;;
> > \
> > for (t.feature = 0; t.feature < FEATURE_COUNT; t.feature++) {
> > \
> > for (t.pipes = 0; t.pipes < PIPE_COUNT; t.pipes++) {
> > \
> > for (t.screen = 0; t.screen < SCREEN_COUNT; t.screen++) {
> > \
> > @@ -3315,6 +3347,7 @@ igt_main_args("", long_options, help_str,
> > opt_handler, NULL)
> > /* Make sure nothing is using these values. */
> > t.flip = -1;
> > t.method = -1;
> > + t.tiling = opt.tiling;
> >
> > igt_subtest_f("%s-%s-rte",
> > feature_str(t.feature),
> > @@ -3499,6 +3532,7 @@ igt_main_args("", long_options, help_str,
> > opt_handler, NULL)
> > t.feature = FEATURE_DEFAULT;
> > t.format = FORMAT_DEFAULT;
> > t.flip = FLIP_PAGEFLIP;
> > + t.tiling = opt.tiling;
> > igt_subtest("basic") {
> > igt_require_gem(drm.fd);
> > basic_subtest(&t);
> >
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev