Re: [Intel-gfx] [PATCH 2/2] kms_rotation_crc: Adding test for 90/270 rotation

Tue, 21 Apr 2015 04:55:52 -0700 


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 <[email protected]>
---
  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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to