Hi Mika, On 15 November 2016 at 12:38, Mika Kahola <mika.kahola@xxxxxxxxx> wrote: > Testcase for plane visibility after atomic modesets. The idea of the test > is the following: > > - draw a blue screen with high resolution > - enable a yellow plane, visible, in lower-left corner > - set a new lower resolution mode (1024x768) that makes plane invisible > - check from debugfs 'i915_display_info' that the plane is invisible > - switch back to higher resolution mode > - check from debugfs 'i915_display_info' that the plane is visible again > - repeat number of iterations, default 64 It would be nice to have this for non-Intel drivers as well, with these suggestions: > +static inline uint32_t pipe_select(int pipe) > +{ > + if (pipe > 1) > + return pipe << DRM_VBLANK_HIGH_CRTC_SHIFT; > + else if (pipe > 0) > + return DRM_VBLANK_SECONDARY; > + else > + return 0; > +} > + > +static unsigned int get_vblank(int fd, int pipe, unsigned int flags) > +{ > + union drm_wait_vblank vbl; > + > + memset(&vbl, 0, sizeof(vbl)); > + vbl.request.type = DRM_VBLANK_RELATIVE | pipe_select(pipe) | flags; > + if (drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl)) > + return 0; > + > + return vbl.reply.sequence; > +} > + > +static int parse_resolution(res_t *resolution, char *info) > +{ > + char size[32]; > + int ret; > + > + ret = sscanf(info + 4, "%d%*c %*s %*s %*s %s%*c", > + &resolution->id, size); > + > + if (ret != 2) > + return -1; > + > + ret = sscanf(size + 6, "%d%*c%d%*c", > + &resolution->width, &resolution->height); > + > + if (ret != 2) > + return -1; > + > + return ret + 1; > +} Could all these be helpers? > +static bool get_visibility(FILE *fid, res_t *resolution) > +{ > + char tmp[256]; > + res_t plane; > + int ret; > + > + while (fgets(tmp, 256, fid) != NULL) { > + if (strstr(tmp, "type=OVL") != NULL) { > + ret = sscanf(tmp + 12, "%d%*c %*s %*s %d%*c%d%*c", > + &plane.id, &plane.width, &plane.height); > + > + igt_assert_eq(ret, 3); > + > + if (plane.width > resolution->width) > + return false; > + else if (plane.height > resolution->height) > + return false; > + else > + return true; > + } > + } > + > + return false; > +} > + > +static bool plane_visible(void) > +{ > + char tmp[256]; > + FILE *fid; > + bool visible = false; > + res_t resolution; > + int ret; > + > + fid = fopen("/sys/kernel/debug/dri/0/i915_display_info", "r"); > + > + if (fid == NULL) > + return false; > + > + while (fgets(tmp, 256, fid) != NULL) { > + if (strstr(tmp, "active=yes") != NULL) { > + ret = parse_resolution(&resolution, tmp); > + igt_assert_eq(ret, 3); > + visible = get_visibility(fid, &resolution); > + } > + } > + > + fclose(fid); > + > + return visible; > +} Could this be made optional, determined from property state? > +static drmModeModeInfo * > +test_setup(data_t *data, enum pipe pipe, uint64_t tiling, int flags, > + igt_output_t *output) > +{ > + drmModeModeInfo *mode; > + int x, y; > + > + igt_output_set_pipe(output, pipe); > + > + data->plane[0] = igt_output_get_plane(output, IGT_PLANE_PRIMARY); > + data->plane[1] = igt_output_get_plane(output, IGT_PLANE_2); Could the test skip if these were unavailable? > + mode = igt_output_get_mode(output); > + > + x = 0; > + y = mode->vdisplay - SIZE; > + > + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay, > + DRM_FORMAT_XRGB8888, > + LOCAL_DRM_FORMAT_MOD_NONE, > + 0.0, 0.0, 1.0, > + &data->fb[0]); > + > + igt_plane_set_fb(data->plane[0], &data->fb[0]); > + > + /* yellow sprite plane in lower left corner */ > + igt_create_color_fb(data->drm_fd, > + SIZE, SIZE, > + DRM_FORMAT_XRGB8888, > + tiling, s/tiling/modifier/ > +static void > +test_plane_position_with_output(data_t *data, enum pipe pipe, > + igt_output_t *output, uint64_t tiling) > +{ > + igt_crc_t *crc_hires1, *crc_hires2; > + igt_crc_t *crc_lowres; > + drmModeModeInfo std_1024_mode = { > + .clock = 65000, > + .hdisplay = 1024, > + .hsync_start = 1048, > + .hsync_end = 1184, > + .htotal = 1344, > + .hskew = 0, > + .vdisplay = 768, > + .vsync_start = 771, > + .vsync_end = 777, > + .vtotal = 806, > + .vscan = 0, > + .vrefresh = 60, > + .flags = 0xA, > + .type = 0x40, > + .name = "Custom 1024x768", > + }; Could this mode only be used as a fallback, with the mode list first being scanned to see if the output natively offers a mode small enough to pass the test? > + i = 0; > + while (i < iterations || loop_forever) { > + /* switch to lower resolution */ > + igt_output_override_mode(output, &std_1024_mode); > + igt_output_set_pipe(output, pipe); > + > + mode2 = igt_output_get_mode(output); > + > + igt_assert_eq(std_1024_mode.hdisplay, mode2->hdisplay); > + igt_assert_eq(std_1024_mode.vdisplay, mode2->vdisplay); > + igt_assert_eq(std_1024_mode.vrefresh, mode2->vrefresh); > + > + display_commit_mode(data, pipe, flags, crc_lowres); Could the test skip if this fails? Either because the output does not support the mode, or because some hardware does not allow planes to be marked as visible but offscreen. > +static void > +run_tests_for_pipe(data_t *data, enum pipe pipe) > +{ > + igt_subtest_f("pipe-%s-tiling-none", > + kmstest_pipe_name(pipe)) > + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_X_TILED); > + > + igt_subtest_f("pipe-%s-tiling-x", > + kmstest_pipe_name(pipe)) > + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_X_TILED); > + > + igt_subtest_f("pipe-%s-tiling-y", > + kmstest_pipe_name(pipe)) > + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Y_TILED); > + > + igt_subtest_f("pipe-%s-tiling-yf", > + kmstest_pipe_name(pipe)) > + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Yf_TILED); > +} Is there any reason to not test MOD_NONE as well? > + igt_fixture { > + data.drm_fd = drm_open_driver_master(DRIVER_INTEL); With all these, you should at least be very close to making this DRIVER_ANY. Cheers, Daniel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx