* [PATCH 1/2] lib/igt_kms: Let set_property return the result @ 2015-04-07 8:29 Sonika Jindal 2015-04-07 8:29 ` [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation Sonika Jindal 2015-04-21 10:40 ` [PATCH 1/2] lib/igt_kms: Let set_property return the result Tvrtko Ursulin 0 siblings, 2 replies; 9+ messages in thread From: Sonika Jindal @ 2015-04-07 8:29 UTC (permalink / raw) To: intel-gfx Return the return value of the set_property ioctl and add check for the failure. Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- lib/igt_kms.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 6cb1f08..14abae8 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -927,13 +927,13 @@ get_plane_property(int drm_fd, uint32_t plane_id, const char *name, name, prop_id, value, prop); } -static void +static int igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value) { igt_pipe_t *pipe = plane->pipe; igt_display_t *display = pipe->display; - drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id, + return drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE, prop_id, value); } @@ -1338,10 +1338,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane, plane->position_changed = false; if (plane->rotation_changed) { - igt_plane_set_property(plane, plane->rotation_property, + ret = igt_plane_set_property(plane, plane->rotation_property, plane->rotation); plane->rotation_changed = false; + CHECK_RETURN(ret, fail_on_error); } return 0; -- 1.7.10.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-07 8:29 [PATCH 1/2] lib/igt_kms: Let set_property return the result Sonika Jindal @ 2015-04-07 8:29 ` Sonika Jindal 2015-04-21 11:54 ` Tvrtko Ursulin 2015-04-21 10:40 ` [PATCH 1/2] lib/igt_kms: Let set_property return the result Tvrtko Ursulin 1 sibling, 1 reply; 9+ messages in thread From: Sonika Jindal @ 2015-04-07 8:29 UTC (permalink / raw) To: intel-gfx Adding 90/270 rotation testcase for primary and sprite planes. v2: Added position test for sprite. Checking for gen > 9 for 90/270. Some cleanup and rebase. v3: Added test for unsupported tiling and unsupported pixel format for 90/270 Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- tests/kms_rotation_crc.c | 248 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 215 insertions(+), 33 deletions(-) diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index e11d7a9..48afaa1 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -28,14 +28,21 @@ #include "igt_debugfs.h" #include "igt_kms.h" #include "igt_core.h" +#include "intel_chipset.h" typedef struct { int gfx_fd; igt_display_t display; struct igt_fb fb; struct igt_fb fb_cursor; + struct igt_fb fb_full; + struct igt_fb fb_565; + struct igt_fb fb_tiling; igt_crc_t ref_crc; igt_pipe_crc_t *pipe_crc; + igt_rotation_t rotation; + int pos_x; + int pos_y; } data_t; static void @@ -63,18 +70,46 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode, w = mode->hdisplay; h = mode->vdisplay; - cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb); + cr = igt_get_cairo_ctx(data->gfx_fd, fb); if (rotation == IGT_ROTATION_180) { cairo_translate(cr, w, h); cairo_rotate(cr, M_PI); } - /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ - igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); - igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); - igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); - igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); + /* + * "rotation" is used for creating ref rotated fb and + * "data->rotation" is used to determine the required size + * while creating unrotated fb. + */ + if (rotation == IGT_ROTATION_90) { + /* Paint 4 squares with width == height in Blue, Red, + Green, White Clockwise order to look like 90 degree rotated*/ + w = h = mode->vdisplay; + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 0.0, 1.0); + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 0.0, 0.0); + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 1.0, 0.0); + + } else if (rotation == IGT_ROTATION_270) { + /* Paint 4 squares with width == height in Green, White, + Blue, Red Clockwise order to look like 270 degree rotated*/ + w = h = mode->vdisplay; + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 1.0, 0.0); + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 1.0, 1.0); + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 0.0, 0.0); + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); + + } else { + if (data->rotation == IGT_ROTATION_90 || + data->rotation == IGT_ROTATION_270) + w = h = mode->vdisplay; + /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ + igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); + } } cairo_destroy(cr); } @@ -84,7 +119,12 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, { drmModeModeInfo *mode; igt_display_t *display = &data->display; - int fb_id, fb_cursor_id; + int fb_id, fb_cursor_id, fb_full_id; + int w, h; + uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE; + enum igt_commit_style commit = COMMIT_LEGACY; + int old_rotation; + igt_plane_t *primary; igt_output_set_pipe(output, pipe); @@ -94,10 +134,45 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + fb_full_id = igt_create_fb(data->gfx_fd, + w, h, + DRM_FORMAT_XRGB8888, + tiling, + &data->fb_full); + igt_assert(fb_full_id); + + /* + * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the + * setplane without a modeset. So, to be able to call + * igt_display_commit and ultimately setcrtc to do the first modeset, + * we create an fb covering the crtc and call commit + */ + + old_rotation = data->rotation; + data->rotation = IGT_ROTATION_0; + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); + paint_squares(data, &data->fb_full, mode, IGT_ROTATION_0, primary); + igt_plane_set_fb(primary, &data->fb_full); + igt_display_commit(display); + data->rotation = old_rotation; + + /* + * For 90/270, we will use create smaller fb so that the rotated + * frame can fit in + */ + if (data->rotation == IGT_ROTATION_90 || + data->rotation == IGT_ROTATION_270) { + tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; + w = h = mode->vdisplay; + } + fb_id = igt_create_fb(data->gfx_fd, - mode->hdisplay, mode->vdisplay, + w, h, DRM_FORMAT_XRGB8888, - LOCAL_DRM_FORMAT_MOD_NONE, + tiling, &data->fb); igt_assert(fb_id); @@ -110,27 +185,20 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, /* Step 1: create a reference CRC for a software-rotated fb */ - /* - * XXX: We always set the primary plane to actually enable the pipe as - * there's no way (that works) to light up a pipe with only a sprite - * plane enabled at the moment. - */ - if (!plane->is_primary) { - igt_plane_t *primary; - - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, primary); - igt_plane_set_fb(primary, &data->fb); - } - if (plane->is_cursor) { - paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_180, plane); + paint_squares(data, &data->fb_cursor, mode, data->rotation, plane); igt_plane_set_fb(plane, &data->fb_cursor); } else { - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, plane); + paint_squares(data, &data->fb, mode, data->rotation, plane); igt_plane_set_fb(plane, &data->fb); + igt_plane_set_position(plane, data->pos_x, data->pos_y); } - igt_display_commit(display); + if (plane->is_primary || plane->is_cursor) { + igt_require(data->display.has_universal_planes); + commit = COMMIT_UNIVERSAL; + } + + igt_display_commit2(display, commit); igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc); /* @@ -155,6 +223,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane) igt_remove_fb(data->gfx_fd, &data->fb); igt_remove_fb(data->gfx_fd, &data->fb_cursor); + igt_remove_fb(data->gfx_fd, &data->fb_full); /* XXX: see the note in prepare_crtc() */ if (!plane->is_primary) { @@ -170,6 +239,69 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane) igt_display_commit(display); } +static void test_unsupported_tiling_pixel_format(data_t *data, enum igt_plane plane_type) +{ + drmModeModeInfo *mode; + igt_display_t *display = &data->display; + igt_output_t *output; + enum pipe pipe; + int valid_tests = 0; + int fb_tiling_id, fb_565_id; + + for_each_connected_output(display, output) { + for_each_pipe(display, pipe) { + igt_plane_t *plane; + + igt_output_set_pipe(output, pipe); + + plane = igt_output_get_plane(output, plane_type); + igt_require(igt_plane_supports_rotation(plane)); + mode = igt_output_get_mode(output); + + fb_tiling_id = igt_create_fb(data->gfx_fd, + mode->vdisplay, mode->hdisplay, + DRM_FORMAT_XRGB8888, + LOCAL_DRM_FORMAT_MOD_NONE, + &data->fb_tiling); + igt_assert(fb_tiling_id); + paint_squares(data, &data->fb_tiling, mode, IGT_ROTATION_0, plane); + igt_plane_set_fb(plane, &data->fb_tiling); + igt_plane_set_rotation(plane, data->rotation); + /* Shud fail because 90/270 is only supported with Y/Yf */ + igt_assert(igt_display_try_commit2(display, COMMIT_UNIVERSAL) == -EINVAL); + + fb_565_id = igt_create_fb(data->gfx_fd, + mode->vdisplay, mode->hdisplay, + DRM_FORMAT_RGB565, + LOCAL_I915_FORMAT_MOD_Y_TILED, + &data->fb_565); + igt_assert(fb_565_id); + paint_squares(data, &data->fb_565, mode, IGT_ROTATION_0, plane); + igt_plane_set_fb(plane, &data->fb_565); + igt_plane_set_rotation(plane, data->rotation); + /* Shud fail because 90/270 is not supported with RGB565 */ + igt_assert(igt_display_try_commit2(display, COMMIT_UNIVERSAL) == -EINVAL); + + /* + * check the rotation state has been reset when the VT + * mode is restored + */ + kmstest_restore_vt_mode(); + kmstest_set_vt_graphics_mode(); + prepare_crtc(data, output, pipe, plane); + + valid_tests++; + + igt_remove_fb(data->gfx_fd, &data->fb_tiling); + igt_remove_fb(data->gfx_fd, &data->fb_565); + cleanup_crtc(data, output, plane); + + igt_display_commit(display); + + } + } +} + static void test_plane_rotation(data_t *data, enum igt_plane plane_type) { igt_display_t *display = &data->display; @@ -200,21 +332,24 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) /* collect unrotated CRC */ igt_pipe_crc_collect_crc(data->pipe_crc, &crc_unrotated); - igt_plane_set_rotation(plane, IGT_ROTATION_180); + igt_plane_set_rotation(plane, data->rotation); igt_display_commit2(display, commit); igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output); + igt_assert_crc_equal(&data->ref_crc, &crc_output); - /* check the rotation state has been reset when the VT - * mode is restored */ + /* + * check the rotation state has been reset when the VT + * mode is restored + */ kmstest_restore_vt_mode(); kmstest_set_vt_graphics_mode(); prepare_crtc(data, output, pipe, plane); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output); igt_assert_crc_equal(&crc_unrotated, &crc_output); - valid_tests++; cleanup_crtc(data, output, plane); } @@ -225,11 +360,13 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) igt_main { data_t data = {}; + int gen = 0; igt_skip_on_simulation(); igt_fixture { data.gfx_fd = drm_open_any_master(); + gen = intel_gen(intel_get_drm_devid(data.gfx_fd)); kmstest_set_vt_graphics_mode(); @@ -237,15 +374,60 @@ igt_main igt_display_init(&data.display, data.gfx_fd); } - - igt_subtest_f("primary-rotation") + igt_subtest_f("primary-rotation-180") { + data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, IGT_PLANE_PRIMARY); + } - igt_subtest_f("sprite-rotation") + igt_subtest_f("sprite-rotation-180") { + data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, IGT_PLANE_2); + } - igt_subtest_f("cursor-rotation") + igt_subtest_f("cursor-rotation-180") { + data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, IGT_PLANE_CURSOR); + } + + igt_subtest_f("primary-rotation-90") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + test_plane_rotation(&data, IGT_PLANE_PRIMARY); + } + + igt_subtest_f("primary-rotation-270") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_270; + test_plane_rotation(&data, IGT_PLANE_PRIMARY); + } + + igt_subtest_f("sprite-rotation-90") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + test_plane_rotation(&data, IGT_PLANE_2); + } + + igt_subtest_f("sprite-rotation-270") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_270; + test_plane_rotation(&data, IGT_PLANE_2); + } + + igt_subtest_f("sprite-rotation-90-pos-100-0") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + data.pos_x = 100, + data.pos_y = 0; + test_plane_rotation(&data, IGT_PLANE_2); + } + + igt_subtest_f("90-rotation-unsupported-tiling-pixel-format") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + data.pos_x = 0, + data.pos_y = 0; + test_unsupported_tiling_pixel_format(&data, IGT_PLANE_PRIMARY); + } igt_fixture { igt_display_fini(&data.display); -- 1.7.10.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-07 8:29 ` [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation Sonika Jindal @ 2015-04-21 11:54 ` Tvrtko Ursulin 2015-04-22 4:51 ` Jindal, Sonika 0 siblings, 1 reply; 9+ messages in thread From: Tvrtko Ursulin @ 2015-04-21 11:54 UTC (permalink / raw) To: Sonika Jindal, intel-gfx Hi, On 04/07/2015 09:29 AM, Sonika Jindal wrote: > Adding 90/270 rotation testcase for primary and sprite planes. > > v2: Added position test for sprite. Checking for gen > 9 for 90/270. > Some cleanup and rebase. > v3: Added test for unsupported tiling and unsupported pixel format for 90/270 > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > tests/kms_rotation_crc.c | 248 ++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 215 insertions(+), 33 deletions(-) > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c > index e11d7a9..48afaa1 100644 > --- a/tests/kms_rotation_crc.c > +++ b/tests/kms_rotation_crc.c > @@ -28,14 +28,21 @@ > #include "igt_debugfs.h" > #include "igt_kms.h" > #include "igt_core.h" > +#include "intel_chipset.h" > > typedef struct { > int gfx_fd; > igt_display_t display; > struct igt_fb fb; > struct igt_fb fb_cursor; > + struct igt_fb fb_full; > + struct igt_fb fb_565; > + struct igt_fb fb_tiling; Could you do with just fb and not having fb_565 and fb_tiling since I think you only use one at a time? > igt_crc_t ref_crc; > igt_pipe_crc_t *pipe_crc; > + igt_rotation_t rotation; > + int pos_x; > + int pos_y; > } data_t; > > static void > @@ -63,18 +70,46 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode, > w = mode->hdisplay; > h = mode->vdisplay; > > - cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb); > + cr = igt_get_cairo_ctx(data->gfx_fd, fb); > > if (rotation == IGT_ROTATION_180) { > cairo_translate(cr, w, h); > cairo_rotate(cr, M_PI); > } > > - /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ > - igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); > - igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); > - igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); > - igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); > + /* > + * "rotation" is used for creating ref rotated fb and > + * "data->rotation" is used to determine the required size > + * while creating unrotated fb. > + */ This comment and the setup below is not the most straightforward to understand. If I got it right you want rotation to draw a rotated image, and effectively a flag to say whether the drawn image is to be over full fb or in a square. Then what about having something like a "square_image" boolean flag instead of two rotation parameters? > + if (rotation == IGT_ROTATION_90) { > + /* Paint 4 squares with width == height in Blue, Red, > + Green, White Clockwise order to look like 90 degree rotated*/ > + w = h = mode->vdisplay; > + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 0.0, 1.0); > + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 0.0, 0.0); > + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); > + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 1.0, 0.0); > + > + } else if (rotation == IGT_ROTATION_270) { > + /* Paint 4 squares with width == height in Green, White, > + Blue, Red Clockwise order to look like 270 degree rotated*/ > + w = h = mode->vdisplay; > + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 1.0, 0.0); > + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 1.0, 1.0); > + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 0.0, 0.0); > + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); > + > + } else { > + if (data->rotation == IGT_ROTATION_90 || > + data->rotation == IGT_ROTATION_270) > + w = h = mode->vdisplay; > + /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ > + igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); > + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); > + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); > + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); > + } > } > cairo_destroy(cr); > } > @@ -84,7 +119,12 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, > { > drmModeModeInfo *mode; > igt_display_t *display = &data->display; > - int fb_id, fb_cursor_id; > + int fb_id, fb_cursor_id, fb_full_id; > + int w, h; > + uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE; > + enum igt_commit_style commit = COMMIT_LEGACY; > + int old_rotation; > + igt_plane_t *primary; > > igt_output_set_pipe(output, pipe); > > @@ -94,10 +134,45 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, > > mode = igt_output_get_mode(output); > > + w = mode->hdisplay; > + h = mode->vdisplay; > + > + fb_full_id = igt_create_fb(data->gfx_fd, > + w, h, > + DRM_FORMAT_XRGB8888, > + tiling, > + &data->fb_full); > + igt_assert(fb_full_id); > + > + /* > + * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the > + * setplane without a modeset. So, to be able to call > + * igt_display_commit and ultimately setcrtc to do the first modeset, > + * we create an fb covering the crtc and call commit > + */ > + > + old_rotation = data->rotation; > + data->rotation = IGT_ROTATION_0; > + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); > + paint_squares(data, &data->fb_full, mode, IGT_ROTATION_0, primary); > + igt_plane_set_fb(primary, &data->fb_full); > + igt_display_commit(display); > + data->rotation = old_rotation; > + > + /* > + * For 90/270, we will use create smaller fb so that the rotated > + * frame can fit in > + */ > + if (data->rotation == IGT_ROTATION_90 || > + data->rotation == IGT_ROTATION_270) { > + tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; > + w = h = mode->vdisplay; > + } > + > fb_id = igt_create_fb(data->gfx_fd, > - mode->hdisplay, mode->vdisplay, > + w, h, > DRM_FORMAT_XRGB8888, > - LOCAL_DRM_FORMAT_MOD_NONE, > + tiling, > &data->fb); > igt_assert(fb_id); > > @@ -110,27 +185,20 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, > > /* Step 1: create a reference CRC for a software-rotated fb */ > > - /* > - * XXX: We always set the primary plane to actually enable the pipe as > - * there's no way (that works) to light up a pipe with only a sprite > - * plane enabled at the moment. > - */ > - if (!plane->is_primary) { > - igt_plane_t *primary; > - > - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); > - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, primary); > - igt_plane_set_fb(primary, &data->fb); > - } > - > if (plane->is_cursor) { > - paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_180, plane); > + paint_squares(data, &data->fb_cursor, mode, data->rotation, plane); > igt_plane_set_fb(plane, &data->fb_cursor); > } else { > - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, plane); > + paint_squares(data, &data->fb, mode, data->rotation, plane); > igt_plane_set_fb(plane, &data->fb); > + igt_plane_set_position(plane, data->pos_x, data->pos_y); > } > - igt_display_commit(display); > + if (plane->is_primary || plane->is_cursor) { > + igt_require(data->display.has_universal_planes); > + commit = COMMIT_UNIVERSAL; > + } > + > + igt_display_commit2(display, commit); > igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc); > > /* > @@ -155,6 +223,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane) > > igt_remove_fb(data->gfx_fd, &data->fb); > igt_remove_fb(data->gfx_fd, &data->fb_cursor); > + igt_remove_fb(data->gfx_fd, &data->fb_full); > > /* XXX: see the note in prepare_crtc() */ > if (!plane->is_primary) { > @@ -170,6 +239,69 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane) > igt_display_commit(display); > } > > +static void test_unsupported_tiling_pixel_format(data_t *data, enum igt_plane plane_type) > +{ If the function body here is essentially the same as test_plane_rotation, could you avoid adding duplicating code by passing in tiling and pixel format to test_plane_rotation (maybe via data)? With a "expect fail" flag, or even just embedding the knowledge would work. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-21 11:54 ` Tvrtko Ursulin @ 2015-04-22 4:51 ` Jindal, Sonika 2015-04-22 9:34 ` Tvrtko Ursulin 0 siblings, 1 reply; 9+ messages in thread From: Jindal, Sonika @ 2015-04-22 4:51 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx On 4/21/2015 5:24 PM, Tvrtko Ursulin wrote: > > Hi, > > On 04/07/2015 09:29 AM, Sonika Jindal wrote: >> Adding 90/270 rotation testcase for primary and sprite planes. >> >> v2: Added position test for sprite. Checking for gen > 9 for 90/270. >> Some cleanup and rebase. >> v3: Added test for unsupported tiling and unsupported pixel format for >> 90/270 >> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >> --- >> tests/kms_rotation_crc.c | 248 >> ++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 215 insertions(+), 33 deletions(-) >> >> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c >> index e11d7a9..48afaa1 100644 >> --- a/tests/kms_rotation_crc.c >> +++ b/tests/kms_rotation_crc.c >> @@ -28,14 +28,21 @@ >> #include "igt_debugfs.h" >> #include "igt_kms.h" >> #include "igt_core.h" >> +#include "intel_chipset.h" >> >> typedef struct { >> int gfx_fd; >> igt_display_t display; >> struct igt_fb fb; >> struct igt_fb fb_cursor; >> + struct igt_fb fb_full; >> + struct igt_fb fb_565; >> + struct igt_fb fb_tiling; > > Could you do with just fb and not having fb_565 and fb_tiling since I > think you only use one at a time? > Yes I can, but then I'l have to create and remove fb during the test. Since this is just one negative test (for 565 format and tiling) I wanted to add, I thought it will look better this way. >> igt_crc_t ref_crc; >> igt_pipe_crc_t *pipe_crc; >> + igt_rotation_t rotation; >> + int pos_x; >> + int pos_y; >> } data_t; >> >> static void >> @@ -63,18 +70,46 @@ paint_squares(data_t *data, struct igt_fb *fb, >> drmModeModeInfo *mode, >> w = mode->hdisplay; >> h = mode->vdisplay; >> >> - cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb); >> + cr = igt_get_cairo_ctx(data->gfx_fd, fb); >> >> if (rotation == IGT_ROTATION_180) { >> cairo_translate(cr, w, h); >> cairo_rotate(cr, M_PI); >> } >> >> - /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ >> - igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); >> - igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); >> - igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); >> - igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); >> + /* >> + * "rotation" is used for creating ref rotated fb and >> + * "data->rotation" is used to determine the required size >> + * while creating unrotated fb. >> + */ > > This comment and the setup below is not the most straightforward to > understand. > > If I got it right you want rotation to draw a rotated image, and > effectively a flag to say whether the drawn image is to be over full fb > or in a square. > > Then what about having something like a "square_image" boolean flag > instead of two rotation parameters? data->rotation has the rotation value which we want our test to rotate the fb with. Now paint_squares gets called two times in a test * First to paint the rotated reference fb to collect the sw rotated crc. For this we set the appropriate rotation in the rotation parameter to paint_squares. (Step 1 in the test) * Next it is called to paint an unrotated fb for the HW to rotate it using set_property. To draw unrotated fb, we pass IGT_ROTATION_0 in rotation parameter to paint_squares.(Step 2 in the test). Only for 90/270, I started using data->rotation inside this function. Otherwise we were not using it. This function is just to paint the fb based upon the rotation. > >> + if (rotation == IGT_ROTATION_90) { >> + /* Paint 4 squares with width == height in Blue, Red, >> + Green, White Clockwise order to look like 90 degree >> rotated*/ >> + w = h = mode->vdisplay; >> + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 0.0, 1.0); >> + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 0.0, 0.0); >> + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); >> + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 1.0, >> 0.0); >> + >> + } else if (rotation == IGT_ROTATION_270) { >> + /* Paint 4 squares with width == height in Green, White, >> + Blue, Red Clockwise order to look like 270 degree rotated*/ >> + w = h = mode->vdisplay; >> + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 1.0, 0.0); >> + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 1.0, 1.0); >> + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 0.0, 0.0); >> + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 0.0, >> 1.0); >> + >> + } else { >> + if (data->rotation == IGT_ROTATION_90 || >> + data->rotation == IGT_ROTATION_270) >> + w = h = mode->vdisplay; >> + /* Paint with 4 squares of Red, Green, White, Blue >> Clockwise */ >> + igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); >> + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); >> + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); >> + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, >> 1.0); >> + } >> } >> cairo_destroy(cr); >> } >> @@ -84,7 +119,12 @@ static void prepare_crtc(data_t *data, >> igt_output_t *output, enum pipe pipe, >> { >> drmModeModeInfo *mode; >> igt_display_t *display = &data->display; >> - int fb_id, fb_cursor_id; >> + int fb_id, fb_cursor_id, fb_full_id; >> + int w, h; >> + uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE; >> + enum igt_commit_style commit = COMMIT_LEGACY; >> + int old_rotation; >> + igt_plane_t *primary; >> >> igt_output_set_pipe(output, pipe); >> >> @@ -94,10 +134,45 @@ static void prepare_crtc(data_t *data, >> igt_output_t *output, enum pipe pipe, >> >> mode = igt_output_get_mode(output); >> >> + w = mode->hdisplay; >> + h = mode->vdisplay; >> + >> + fb_full_id = igt_create_fb(data->gfx_fd, >> + w, h, >> + DRM_FORMAT_XRGB8888, >> + tiling, >> + &data->fb_full); >> + igt_assert(fb_full_id); >> + >> + /* >> + * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the >> + * setplane without a modeset. So, to be able to call >> + * igt_display_commit and ultimately setcrtc to do the first >> modeset, >> + * we create an fb covering the crtc and call commit >> + */ >> + >> + old_rotation = data->rotation; >> + data->rotation = IGT_ROTATION_0; >> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); >> + paint_squares(data, &data->fb_full, mode, IGT_ROTATION_0, primary); >> + igt_plane_set_fb(primary, &data->fb_full); >> + igt_display_commit(display); >> + data->rotation = old_rotation; >> + >> + /* >> + * For 90/270, we will use create smaller fb so that the rotated >> + * frame can fit in >> + */ >> + if (data->rotation == IGT_ROTATION_90 || >> + data->rotation == IGT_ROTATION_270) { >> + tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; >> + w = h = mode->vdisplay; >> + } >> + >> fb_id = igt_create_fb(data->gfx_fd, >> - mode->hdisplay, mode->vdisplay, >> + w, h, >> DRM_FORMAT_XRGB8888, >> - LOCAL_DRM_FORMAT_MOD_NONE, >> + tiling, >> &data->fb); >> igt_assert(fb_id); >> >> @@ -110,27 +185,20 @@ static void prepare_crtc(data_t *data, >> igt_output_t *output, enum pipe pipe, >> >> /* Step 1: create a reference CRC for a software-rotated fb */ >> >> - /* >> - * XXX: We always set the primary plane to actually enable the >> pipe as >> - * there's no way (that works) to light up a pipe with only a sprite >> - * plane enabled at the moment. >> - */ >> - if (!plane->is_primary) { >> - igt_plane_t *primary; >> - >> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); >> - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, primary); >> - igt_plane_set_fb(primary, &data->fb); >> - } >> - >> if (plane->is_cursor) { >> - paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_180, >> plane); >> + paint_squares(data, &data->fb_cursor, mode, data->rotation, >> plane); >> igt_plane_set_fb(plane, &data->fb_cursor); >> } else { >> - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, plane); >> + paint_squares(data, &data->fb, mode, data->rotation, plane); >> igt_plane_set_fb(plane, &data->fb); >> + igt_plane_set_position(plane, data->pos_x, data->pos_y); >> } >> - igt_display_commit(display); >> + if (plane->is_primary || plane->is_cursor) { >> + igt_require(data->display.has_universal_planes); >> + commit = COMMIT_UNIVERSAL; >> + } >> + >> + igt_display_commit2(display, commit); >> igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc); >> >> /* >> @@ -155,6 +223,7 @@ static void cleanup_crtc(data_t *data, >> igt_output_t *output, igt_plane_t *plane) >> >> igt_remove_fb(data->gfx_fd, &data->fb); >> igt_remove_fb(data->gfx_fd, &data->fb_cursor); >> + igt_remove_fb(data->gfx_fd, &data->fb_full); >> >> /* XXX: see the note in prepare_crtc() */ >> if (!plane->is_primary) { >> @@ -170,6 +239,69 @@ static void cleanup_crtc(data_t *data, >> igt_output_t *output, igt_plane_t *plane) >> igt_display_commit(display); >> } >> >> +static void test_unsupported_tiling_pixel_format(data_t *data, enum >> igt_plane plane_type) >> +{ > > If the function body here is essentially the same as > test_plane_rotation, could you avoid adding duplicating code by passing > in tiling and pixel format to test_plane_rotation (maybe via data)? With > a "expect fail" flag, or even just embedding the knowledge would work. > They are not same. We don't need few things from test_plane_rotation in this one like prepare_crtc. So, instead of using if/else at multiple places, I find it cleaner to have a different function for negative tests. Regards, Sonika > Regards, > > Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-22 4:51 ` Jindal, Sonika @ 2015-04-22 9:34 ` Tvrtko Ursulin 2015-04-22 11:14 ` [PATCH] " Sonika Jindal 0 siblings, 1 reply; 9+ messages in thread From: Tvrtko Ursulin @ 2015-04-22 9:34 UTC (permalink / raw) To: Jindal, Sonika, intel-gfx On 04/22/2015 05:51 AM, Jindal, Sonika wrote: >> On 04/07/2015 09:29 AM, Sonika Jindal wrote: >>> Adding 90/270 rotation testcase for primary and sprite planes. >>> >>> v2: Added position test for sprite. Checking for gen > 9 for 90/270. >>> Some cleanup and rebase. >>> v3: Added test for unsupported tiling and unsupported pixel format for >>> 90/270 >>> >>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >>> --- >>> tests/kms_rotation_crc.c | 248 >>> ++++++++++++++++++++++++++++++++++++++++------ >>> 1 file changed, 215 insertions(+), 33 deletions(-) >>> >>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c >>> index e11d7a9..48afaa1 100644 >>> --- a/tests/kms_rotation_crc.c >>> +++ b/tests/kms_rotation_crc.c >>> @@ -28,14 +28,21 @@ >>> #include "igt_debugfs.h" >>> #include "igt_kms.h" >>> #include "igt_core.h" >>> +#include "intel_chipset.h" >>> >>> typedef struct { >>> int gfx_fd; >>> igt_display_t display; >>> struct igt_fb fb; >>> struct igt_fb fb_cursor; >>> + struct igt_fb fb_full; >>> + struct igt_fb fb_565; >>> + struct igt_fb fb_tiling; >> >> Could you do with just fb and not having fb_565 and fb_tiling since I >> think you only use one at a time? >> > Yes I can, but then I'l have to create and remove fb during the test. > Since this is just one negative test (for 565 format and tiling) I > wanted to add, I thought it will look better this way. I think they should go since, if you look, then don't belong to data at all, no code takes data and uses them from there but only the negative tests uses them only locally. So I think local igt_fb in test_unsupported_tiling_pixel_format would be more appropriate. >>> igt_crc_t ref_crc; >>> igt_pipe_crc_t *pipe_crc; >>> + igt_rotation_t rotation; >>> + int pos_x; >>> + int pos_y; >>> } data_t; >>> >>> static void >>> @@ -63,18 +70,46 @@ paint_squares(data_t *data, struct igt_fb *fb, >>> drmModeModeInfo *mode, >>> w = mode->hdisplay; >>> h = mode->vdisplay; >>> >>> - cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb); >>> + cr = igt_get_cairo_ctx(data->gfx_fd, fb); >>> >>> if (rotation == IGT_ROTATION_180) { >>> cairo_translate(cr, w, h); >>> cairo_rotate(cr, M_PI); >>> } >>> >>> - /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ >>> - igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); >>> - igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); >>> - igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); >>> - igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); >>> + /* >>> + * "rotation" is used for creating ref rotated fb and >>> + * "data->rotation" is used to determine the required size >>> + * while creating unrotated fb. >>> + */ >> >> This comment and the setup below is not the most straightforward to >> understand. >> >> If I got it right you want rotation to draw a rotated image, and >> effectively a flag to say whether the drawn image is to be over full fb >> or in a square. >> >> Then what about having something like a "square_image" boolean flag >> instead of two rotation parameters? > data->rotation has the rotation value which we want our test to rotate > the fb with. > Now paint_squares gets called two times in a test > * First to paint the rotated reference fb to collect the sw rotated crc. > For this we set the appropriate rotation in the rotation parameter to > paint_squares. (Step 1 in the test) > * Next it is called to paint an unrotated fb for the HW to rotate it > using set_property. To draw unrotated fb, we pass IGT_ROTATION_0 in > rotation parameter to paint_squares.(Step 2 in the test). > > Only for 90/270, I started using data->rotation inside this function. > Otherwise we were not using it. > This function is just to paint the fb based upon the rotation. Yes I get what it does :) my point was that it uses the second rotation to decide whether to paint a rectangular (full fb) or square (sub-fb) image and that it would be more obvious if the parameter said so. >> >>> + if (rotation == IGT_ROTATION_90) { >>> + /* Paint 4 squares with width == height in Blue, Red, >>> + Green, White Clockwise order to look like 90 degree >>> rotated*/ >>> + w = h = mode->vdisplay; >>> + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 0.0, 1.0); >>> + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 0.0, 0.0); >>> + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); >>> + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 1.0, >>> 0.0); >>> + >>> + } else if (rotation == IGT_ROTATION_270) { >>> + /* Paint 4 squares with width == height in Green, White, >>> + Blue, Red Clockwise order to look like 270 degree rotated*/ >>> + w = h = mode->vdisplay; >>> + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 1.0, 0.0); >>> + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 1.0, 1.0); >>> + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 0.0, 0.0); >>> + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 0.0, >>> 1.0); >>> + >>> + } else { >>> + if (data->rotation == IGT_ROTATION_90 || >>> + data->rotation == IGT_ROTATION_270) >>> + w = h = mode->vdisplay; >>> + /* Paint with 4 squares of Red, Green, White, Blue >>> Clockwise */ >>> + igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); >>> + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); >>> + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); >>> + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, >>> 1.0); >>> + } >>> } >>> cairo_destroy(cr); >>> } >>> @@ -84,7 +119,12 @@ static void prepare_crtc(data_t *data, >>> igt_output_t *output, enum pipe pipe, >>> { >>> drmModeModeInfo *mode; >>> igt_display_t *display = &data->display; >>> - int fb_id, fb_cursor_id; >>> + int fb_id, fb_cursor_id, fb_full_id; >>> + int w, h; >>> + uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE; >>> + enum igt_commit_style commit = COMMIT_LEGACY; >>> + int old_rotation; >>> + igt_plane_t *primary; >>> >>> igt_output_set_pipe(output, pipe); >>> >>> @@ -94,10 +134,45 @@ static void prepare_crtc(data_t *data, >>> igt_output_t *output, enum pipe pipe, >>> >>> mode = igt_output_get_mode(output); >>> >>> + w = mode->hdisplay; >>> + h = mode->vdisplay; >>> + >>> + fb_full_id = igt_create_fb(data->gfx_fd, >>> + w, h, >>> + DRM_FORMAT_XRGB8888, >>> + tiling, >>> + &data->fb_full); >>> + igt_assert(fb_full_id); >>> + >>> + /* >>> + * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the >>> + * setplane without a modeset. So, to be able to call >>> + * igt_display_commit and ultimately setcrtc to do the first >>> modeset, >>> + * we create an fb covering the crtc and call commit >>> + */ >>> + >>> + old_rotation = data->rotation; >>> + data->rotation = IGT_ROTATION_0; >>> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); >>> + paint_squares(data, &data->fb_full, mode, IGT_ROTATION_0, primary); >>> + igt_plane_set_fb(primary, &data->fb_full); >>> + igt_display_commit(display); >>> + data->rotation = old_rotation; >>> + >>> + /* >>> + * For 90/270, we will use create smaller fb so that the rotated >>> + * frame can fit in >>> + */ >>> + if (data->rotation == IGT_ROTATION_90 || >>> + data->rotation == IGT_ROTATION_270) { >>> + tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; >>> + w = h = mode->vdisplay; >>> + } >>> + >>> fb_id = igt_create_fb(data->gfx_fd, >>> - mode->hdisplay, mode->vdisplay, >>> + w, h, >>> DRM_FORMAT_XRGB8888, >>> - LOCAL_DRM_FORMAT_MOD_NONE, >>> + tiling, >>> &data->fb); >>> igt_assert(fb_id); >>> >>> @@ -110,27 +185,20 @@ static void prepare_crtc(data_t *data, >>> igt_output_t *output, enum pipe pipe, >>> >>> /* Step 1: create a reference CRC for a software-rotated fb */ >>> >>> - /* >>> - * XXX: We always set the primary plane to actually enable the >>> pipe as >>> - * there's no way (that works) to light up a pipe with only a >>> sprite >>> - * plane enabled at the moment. >>> - */ >>> - if (!plane->is_primary) { >>> - igt_plane_t *primary; >>> - >>> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); >>> - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, >>> primary); >>> - igt_plane_set_fb(primary, &data->fb); >>> - } >>> - >>> if (plane->is_cursor) { >>> - paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_180, >>> plane); >>> + paint_squares(data, &data->fb_cursor, mode, data->rotation, >>> plane); >>> igt_plane_set_fb(plane, &data->fb_cursor); >>> } else { >>> - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, plane); >>> + paint_squares(data, &data->fb, mode, data->rotation, plane); >>> igt_plane_set_fb(plane, &data->fb); >>> + igt_plane_set_position(plane, data->pos_x, data->pos_y); >>> } >>> - igt_display_commit(display); >>> + if (plane->is_primary || plane->is_cursor) { >>> + igt_require(data->display.has_universal_planes); >>> + commit = COMMIT_UNIVERSAL; >>> + } >>> + >>> + igt_display_commit2(display, commit); >>> igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc); >>> >>> /* >>> @@ -155,6 +223,7 @@ static void cleanup_crtc(data_t *data, >>> igt_output_t *output, igt_plane_t *plane) >>> >>> igt_remove_fb(data->gfx_fd, &data->fb); >>> igt_remove_fb(data->gfx_fd, &data->fb_cursor); >>> + igt_remove_fb(data->gfx_fd, &data->fb_full); >>> >>> /* XXX: see the note in prepare_crtc() */ >>> if (!plane->is_primary) { >>> @@ -170,6 +239,69 @@ static void cleanup_crtc(data_t *data, >>> igt_output_t *output, igt_plane_t *plane) >>> igt_display_commit(display); >>> } >>> >>> +static void test_unsupported_tiling_pixel_format(data_t *data, enum >>> igt_plane plane_type) >>> +{ >> >> If the function body here is essentially the same as >> test_plane_rotation, could you avoid adding duplicating code by passing >> in tiling and pixel format to test_plane_rotation (maybe via data)? With >> a "expect fail" flag, or even just embedding the knowledge would work. >> > They are not same. We don't need few things from test_plane_rotation in > this one like prepare_crtc. So, instead of using if/else at multiple > places, I find it cleaner to have a different function for negative tests. To me that's marginal since I think they could be identical with just one if/else when it checks what to assert for. But never mind. Bigger issue I have is negative tests fail on my box. :) Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-22 9:34 ` Tvrtko Ursulin @ 2015-04-22 11:14 ` Sonika Jindal 2015-04-22 13:37 ` Tvrtko Ursulin 0 siblings, 1 reply; 9+ messages in thread From: Sonika Jindal @ 2015-04-22 11:14 UTC (permalink / raw) To: intel-gfx Adding 90/270 rotation testcase for primary and sprite planes. v2: Added position test for sprite. Checking for gen > 9 for 90/270. Some cleanup and rebase. v3: Added test for unsupported tiling and unsupported pixel format for 90/270 v4: Added the legacy commit to initiate modeset in the negative test(Tvrtko) Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- tests/kms_rotation_crc.c | 250 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 217 insertions(+), 33 deletions(-) diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index e11d7a9..eda66de 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -28,14 +28,21 @@ #include "igt_debugfs.h" #include "igt_kms.h" #include "igt_core.h" +#include "intel_chipset.h" typedef struct { int gfx_fd; igt_display_t display; struct igt_fb fb; struct igt_fb fb_cursor; + struct igt_fb fb_full; + struct igt_fb fb_565; + struct igt_fb fb_tiling; igt_crc_t ref_crc; igt_pipe_crc_t *pipe_crc; + igt_rotation_t rotation; + int pos_x; + int pos_y; } data_t; static void @@ -63,18 +70,46 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode, w = mode->hdisplay; h = mode->vdisplay; - cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb); + cr = igt_get_cairo_ctx(data->gfx_fd, fb); if (rotation == IGT_ROTATION_180) { cairo_translate(cr, w, h); cairo_rotate(cr, M_PI); } - /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ - igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); - igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); - igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); - igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); + /* + * "rotation" is used for creating ref rotated fb and + * "data->rotation" is used to determine the required size + * while creating unrotated fb. + */ + if (rotation == IGT_ROTATION_90) { + /* Paint 4 squares with width == height in Blue, Red, + Green, White Clockwise order to look like 90 degree rotated*/ + w = h = mode->vdisplay; + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 0.0, 1.0); + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 0.0, 0.0); + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 1.0, 0.0); + + } else if (rotation == IGT_ROTATION_270) { + /* Paint 4 squares with width == height in Green, White, + Blue, Red Clockwise order to look like 270 degree rotated*/ + w = h = mode->vdisplay; + igt_paint_color(cr, 0, 0, w / 2, h / 2, 0.0, 1.0, 0.0); + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 1.0, 1.0, 1.0); + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 1.0, 0.0, 0.0); + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); + + } else { + if (data->rotation == IGT_ROTATION_90 || + data->rotation == IGT_ROTATION_270) + w = h = mode->vdisplay; + /* Paint with 4 squares of Red, Green, White, Blue Clockwise */ + igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0); + igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0); + igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0); + igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0); + } } cairo_destroy(cr); } @@ -84,7 +119,12 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, { drmModeModeInfo *mode; igt_display_t *display = &data->display; - int fb_id, fb_cursor_id; + int fb_id, fb_cursor_id, fb_full_id; + int w, h; + uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE; + enum igt_commit_style commit = COMMIT_LEGACY; + int old_rotation; + igt_plane_t *primary; igt_output_set_pipe(output, pipe); @@ -94,10 +134,45 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + fb_full_id = igt_create_fb(data->gfx_fd, + w, h, + DRM_FORMAT_XRGB8888, + tiling, + &data->fb_full); + igt_assert(fb_full_id); + + /* + * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the + * setplane without a modeset. So, to be able to call + * igt_display_commit and ultimately setcrtc to do the first modeset, + * we create an fb covering the crtc and call commit + */ + + old_rotation = data->rotation; + data->rotation = IGT_ROTATION_0; + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); + paint_squares(data, &data->fb_full, mode, IGT_ROTATION_0, primary); + igt_plane_set_fb(primary, &data->fb_full); + igt_display_commit(display); + data->rotation = old_rotation; + + /* + * For 90/270, we will use create smaller fb so that the rotated + * frame can fit in + */ + if (data->rotation == IGT_ROTATION_90 || + data->rotation == IGT_ROTATION_270) { + tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; + w = h = mode->vdisplay; + } + fb_id = igt_create_fb(data->gfx_fd, - mode->hdisplay, mode->vdisplay, + w, h, DRM_FORMAT_XRGB8888, - LOCAL_DRM_FORMAT_MOD_NONE, + tiling, &data->fb); igt_assert(fb_id); @@ -110,27 +185,20 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, /* Step 1: create a reference CRC for a software-rotated fb */ - /* - * XXX: We always set the primary plane to actually enable the pipe as - * there's no way (that works) to light up a pipe with only a sprite - * plane enabled at the moment. - */ - if (!plane->is_primary) { - igt_plane_t *primary; - - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, primary); - igt_plane_set_fb(primary, &data->fb); - } - if (plane->is_cursor) { - paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_180, plane); + paint_squares(data, &data->fb_cursor, mode, data->rotation, plane); igt_plane_set_fb(plane, &data->fb_cursor); } else { - paint_squares(data, &data->fb, mode, IGT_ROTATION_180, plane); + paint_squares(data, &data->fb, mode, data->rotation, plane); igt_plane_set_fb(plane, &data->fb); + igt_plane_set_position(plane, data->pos_x, data->pos_y); } - igt_display_commit(display); + if (plane->is_primary || plane->is_cursor) { + igt_require(data->display.has_universal_planes); + commit = COMMIT_UNIVERSAL; + } + + igt_display_commit2(display, commit); igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc); /* @@ -155,6 +223,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane) igt_remove_fb(data->gfx_fd, &data->fb); igt_remove_fb(data->gfx_fd, &data->fb_cursor); + igt_remove_fb(data->gfx_fd, &data->fb_full); /* XXX: see the note in prepare_crtc() */ if (!plane->is_primary) { @@ -170,6 +239,71 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane) igt_display_commit(display); } +static void test_unsupported_tiling_pixel_format(data_t *data, enum igt_plane plane_type) +{ + drmModeModeInfo *mode; + igt_display_t *display = &data->display; + igt_output_t *output; + enum pipe pipe; + int valid_tests = 0; + int fb_tiling_id, fb_565_id; + + for_each_connected_output(display, output) { + for_each_pipe(display, pipe) { + igt_plane_t *plane; + + igt_output_set_pipe(output, pipe); + + plane = igt_output_get_plane(output, plane_type); + igt_require(igt_plane_supports_rotation(plane)); + mode = igt_output_get_mode(output); + + fb_tiling_id = igt_create_fb(data->gfx_fd, + mode->hdisplay, mode->vdisplay, + DRM_FORMAT_XRGB8888, + LOCAL_DRM_FORMAT_MOD_NONE, + &data->fb_tiling); + igt_assert(fb_tiling_id); + paint_squares(data, &data->fb_tiling, mode, IGT_ROTATION_0, plane); + igt_plane_set_fb(plane, &data->fb_tiling); + /* For the first modeset with legacy commit */ + igt_display_commit(display); + igt_plane_set_rotation(plane, data->rotation); + /* Shud fail because 90/270 is only supported with Y/Yf */ + igt_assert(igt_display_try_commit2(display, COMMIT_UNIVERSAL) == -EINVAL); + + fb_565_id = igt_create_fb(data->gfx_fd, + mode->hdisplay, mode->vdisplay, + DRM_FORMAT_RGB565, + LOCAL_I915_FORMAT_MOD_Y_TILED, + &data->fb_565); + igt_assert(fb_565_id); + paint_squares(data, &data->fb_565, mode, IGT_ROTATION_0, plane); + igt_plane_set_fb(plane, &data->fb_565); + igt_plane_set_rotation(plane, data->rotation); + /* Shud fail because 90/270 is not supported with RGB565 */ + igt_assert(igt_display_try_commit2(display, COMMIT_UNIVERSAL) == -EINVAL); + + /* + * check the rotation state has been reset when the VT + * mode is restored + */ + kmstest_restore_vt_mode(); + kmstest_set_vt_graphics_mode(); + prepare_crtc(data, output, pipe, plane); + + valid_tests++; + + igt_remove_fb(data->gfx_fd, &data->fb_tiling); + igt_remove_fb(data->gfx_fd, &data->fb_565); + cleanup_crtc(data, output, plane); + + igt_display_commit(display); + + } + } +} + static void test_plane_rotation(data_t *data, enum igt_plane plane_type) { igt_display_t *display = &data->display; @@ -200,21 +334,24 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) /* collect unrotated CRC */ igt_pipe_crc_collect_crc(data->pipe_crc, &crc_unrotated); - igt_plane_set_rotation(plane, IGT_ROTATION_180); + igt_plane_set_rotation(plane, data->rotation); igt_display_commit2(display, commit); igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output); + igt_assert_crc_equal(&data->ref_crc, &crc_output); - /* check the rotation state has been reset when the VT - * mode is restored */ + /* + * check the rotation state has been reset when the VT + * mode is restored + */ kmstest_restore_vt_mode(); kmstest_set_vt_graphics_mode(); prepare_crtc(data, output, pipe, plane); + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output); igt_assert_crc_equal(&crc_unrotated, &crc_output); - valid_tests++; cleanup_crtc(data, output, plane); } @@ -225,11 +362,13 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) igt_main { data_t data = {}; + int gen = 0; igt_skip_on_simulation(); igt_fixture { data.gfx_fd = drm_open_any_master(); + gen = intel_gen(intel_get_drm_devid(data.gfx_fd)); kmstest_set_vt_graphics_mode(); @@ -237,15 +376,60 @@ igt_main igt_display_init(&data.display, data.gfx_fd); } - - igt_subtest_f("primary-rotation") + igt_subtest_f("primary-rotation-180") { + data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, IGT_PLANE_PRIMARY); + } - igt_subtest_f("sprite-rotation") + igt_subtest_f("sprite-rotation-180") { + data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, IGT_PLANE_2); + } - igt_subtest_f("cursor-rotation") + igt_subtest_f("cursor-rotation-180") { + data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, IGT_PLANE_CURSOR); + } + + igt_subtest_f("primary-rotation-90") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + test_plane_rotation(&data, IGT_PLANE_PRIMARY); + } + + igt_subtest_f("primary-rotation-270") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_270; + test_plane_rotation(&data, IGT_PLANE_PRIMARY); + } + + igt_subtest_f("sprite-rotation-90") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + test_plane_rotation(&data, IGT_PLANE_2); + } + + igt_subtest_f("sprite-rotation-270") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_270; + test_plane_rotation(&data, IGT_PLANE_2); + } + + igt_subtest_f("sprite-rotation-90-pos-100-0") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + data.pos_x = 100, + data.pos_y = 0; + test_plane_rotation(&data, IGT_PLANE_2); + } + + igt_subtest_f("90-rotation-unsupported-tiling-pixel-format") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + data.pos_x = 0, + data.pos_y = 0; + test_unsupported_tiling_pixel_format(&data, IGT_PLANE_PRIMARY); + } igt_fixture { igt_display_fini(&data.display); -- 1.7.10.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-22 11:14 ` [PATCH] " Sonika Jindal @ 2015-04-22 13:37 ` Tvrtko Ursulin 2015-04-22 17:20 ` Thomas Wood 0 siblings, 1 reply; 9+ messages in thread From: Tvrtko Ursulin @ 2015-04-22 13:37 UTC (permalink / raw) To: Sonika Jindal, intel-gfx Hi, On 04/22/2015 12:14 PM, Sonika Jindal wrote: > Adding 90/270 rotation testcase for primary and sprite planes. > > v2: Added position test for sprite. Checking for gen > 9 for 90/270. > Some cleanup and rebase. > v3: Added test for unsupported tiling and unsupported pixel format for 90/270 > v4: Added the legacy commit to initiate modeset in the negative test(Tvrtko) > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > tests/kms_rotation_crc.c | 250 ++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 217 insertions(+), 33 deletions(-) I'll attempt some cleanups while adding flip tests but until then I think this is good enough for a test: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kms_rotation_crc: Adding test for 90/270 rotation 2015-04-22 13:37 ` Tvrtko Ursulin @ 2015-04-22 17:20 ` Thomas Wood 0 siblings, 0 replies; 9+ messages in thread From: Thomas Wood @ 2015-04-22 17:20 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel Graphics Development On 22 April 2015 at 14:37, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > Hi, > > On 04/22/2015 12:14 PM, Sonika Jindal wrote: >> >> Adding 90/270 rotation testcase for primary and sprite planes. >> >> v2: Added position test for sprite. Checking for gen > 9 for 90/270. >> Some cleanup and rebase. >> v3: Added test for unsupported tiling and unsupported pixel format for >> 90/270 >> v4: Added the legacy commit to initiate modeset in the negative >> test(Tvrtko) >> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >> --- >> tests/kms_rotation_crc.c | 250 >> ++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 217 insertions(+), 33 deletions(-) > > > I'll attempt some cleanups while adding flip tests but until then I think > this is good enough for a test: > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Thanks, I've pushed both patches from this series. > > Regards, > > Tvrtko > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] lib/igt_kms: Let set_property return the result 2015-04-07 8:29 [PATCH 1/2] lib/igt_kms: Let set_property return the result Sonika Jindal 2015-04-07 8:29 ` [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation Sonika Jindal @ 2015-04-21 10:40 ` Tvrtko Ursulin 1 sibling, 0 replies; 9+ messages in thread From: Tvrtko Ursulin @ 2015-04-21 10:40 UTC (permalink / raw) To: Sonika Jindal, intel-gfx On 04/07/2015 09:29 AM, Sonika Jindal wrote: > Return the return value of the set_property ioctl and add check for > the failure. > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > lib/igt_kms.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 6cb1f08..14abae8 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -927,13 +927,13 @@ get_plane_property(int drm_fd, uint32_t plane_id, const char *name, > name, prop_id, value, prop); > } > > -static void > +static int > igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value) > { > igt_pipe_t *pipe = plane->pipe; > igt_display_t *display = pipe->display; > > - drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id, > + return drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id, > DRM_MODE_OBJECT_PLANE, prop_id, value); > } > > @@ -1338,10 +1338,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane, > plane->position_changed = false; > > if (plane->rotation_changed) { > - igt_plane_set_property(plane, plane->rotation_property, > + ret = igt_plane_set_property(plane, plane->rotation_property, > plane->rotation); > > plane->rotation_changed = false; > + CHECK_RETURN(ret, fail_on_error); > } > > return 0; > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-22 17:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-04-07 8:29 [PATCH 1/2] lib/igt_kms: Let set_property return the result Sonika Jindal 2015-04-07 8:29 ` [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation Sonika Jindal 2015-04-21 11:54 ` Tvrtko Ursulin 2015-04-22 4:51 ` Jindal, Sonika 2015-04-22 9:34 ` Tvrtko Ursulin 2015-04-22 11:14 ` [PATCH] " Sonika Jindal 2015-04-22 13:37 ` Tvrtko Ursulin 2015-04-22 17:20 ` Thomas Wood 2015-04-21 10:40 ` [PATCH 1/2] lib/igt_kms: Let set_property return the result Tvrtko Ursulin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.