[Intel-gfx] [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Tue Apr 21 04:54:58 PDT 2015
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 at 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
More information about the Intel-gfx
mailing list