On 9/2/22 11:53, Michał Winiarski wrote:
> On Thu, Sep 01, 2022 at 09:20:55AM -0300, Maíra Canal wrote:
>> Hi Michał
>>
>> On 8/31/22 17:45, Michał Winiarski wrote:
>>> The test was constructed as a single function (test case) which checks
>>> multiple conditions, calling the function that is tested multiple times
>>> with different arguments.
>>> This usually means that it can be easily converted into multiple test
>>> cases.
>>> Split igt_check_plane_state into two parameterized test cases,
>>> drm_check_plane_state and drm_check_invalid_plane_state.
>>>
>>> Passing output:
>>> ============================================================
>>> ============== drm_plane_helper (2 subtests) ===============
>>> ================== drm_check_plane_state ===================
>>> [PASSED] clipping_simple
>>> [PASSED] clipping_rotate_reflect
>>> [PASSED] positioning_simple
>>> [PASSED] upscaling
>>> [PASSED] downscaling
>>> [PASSED] rounding1
>>> [PASSED] rounding2
>>> [PASSED] rounding3
>>> [PASSED] rounding4
>>> ============== [PASSED] drm_check_plane_state ==============
>>> ============== drm_check_invalid_plane_state ===============
>>> [PASSED] positioning_invalid
>>> [PASSED] upscaling_invalid
>>> [PASSED] downscaling_invalid
>>> ========== [PASSED] drm_check_invalid_plane_state ==========
>>> ================ [PASSED] drm_plane_helper =================
>>> ============================================================
>>> Testing complete. Ran 12 tests: passed: 12
>>>
>>> v2: Add missing EXPECT/ASSERT (Maíra)
>>>
>>> Signed-off-by: Michał Winiarski <michal.winiar...@intel.com>
>>> ---
>>>  drivers/gpu/drm/tests/drm_plane_helper_test.c | 456 ++++++++++--------
>>>  1 file changed, 267 insertions(+), 189 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tests/drm_plane_helper_test.c 
>>> b/drivers/gpu/drm/tests/drm_plane_helper_test.c
>>> index 0bbd42d2d37b..505173b019d7 100644
>>> --- a/drivers/gpu/drm/tests/drm_plane_helper_test.c
>>> +++ b/drivers/gpu/drm/tests/drm_plane_helper_test.c
>>> @@ -12,59 +12,97 @@
>>>  #include <drm/drm_modes.h>
>>>  #include <drm/drm_rect.h>
>>>  
>>> -static void set_src(struct drm_plane_state *plane_state,
>>> -               unsigned int src_x, unsigned int src_y,
>>> -               unsigned int src_w, unsigned int src_h)
>>> +static const struct drm_crtc_state crtc_state = {
>>> +   .crtc = ZERO_SIZE_PTR,
>>> +   .enable = true,
>>> +   .active = true,
>>> +   .mode = {
>>> +           DRM_MODE("1024x768", 0, 65000, 1024, 1048,
>>> +                    1184, 1344, 0, 768, 771, 777, 806, 0,
>>> +                    DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
>>> +   },
>>> +};
>>> +
>>> +struct drm_check_plane_state_test {
>>> +   const char *name;
>>> +   const char *msg;
>>> +   struct {
>>> +           unsigned int x;
>>> +           unsigned int y;
>>> +           unsigned int w;
>>> +           unsigned int h;
>>> +   } src, src_expected;
>>> +   struct {
>>> +           int x;
>>> +           int y;
>>> +           unsigned int w;
>>> +           unsigned int h;
>>> +   } crtc, crtc_expected;
>>> +   unsigned int rotation;
>>> +   int min_scale;
>>> +   int max_scale;
>>> +   bool can_position;
>>> +};
>>> +
>>> +static int drm_plane_helper_init(struct kunit *test)
>>>  {
>>> -   plane_state->src_x = src_x;
>>> -   plane_state->src_y = src_y;
>>> -   plane_state->src_w = src_w;
>>> -   plane_state->src_h = src_h;
>>> +   const struct drm_check_plane_state_test *params = test->param_value;
>>> +   struct drm_plane *plane;
>>> +   struct drm_framebuffer *fb;
>>> +   struct drm_plane_state *mock;
>>> +
>>> +   plane = kunit_kzalloc(test, sizeof(*plane), GFP_KERNEL);
>>> +   KUNIT_ASSERT_NOT_NULL(test, plane);
>>> +
>>> +   fb = kunit_kzalloc(test, sizeof(*fb), GFP_KERNEL);
>>> +   KUNIT_ASSERT_NOT_NULL(test, fb);
>>> +   fb->width = 2048;
>>> +   fb->height = 2048;
>>> +
>>> +   mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
>>> +   KUNIT_ASSERT_NOT_NULL(test, mock);
>>> +   mock->plane = plane;
>>> +   mock->crtc = ZERO_SIZE_PTR;
>>> +   mock->fb = fb;
>>> +   mock->rotation = params->rotation;
>>> +   mock->src_x = params->src.x;
>>> +   mock->src_y = params->src.y;
>>> +   mock->src_w = params->src.w;
>>> +   mock->src_h = params->src.h;
>>> +   mock->crtc_x = params->crtc.x;
>>> +   mock->crtc_y = params->crtc.y;
>>> +   mock->crtc_w = params->crtc.w;
>>> +   mock->crtc_h = params->crtc.h;
>>> +
>>> +   test->priv = mock;
>>> +
>>> +   return 0;
>>>  }
>>>  
>>> -static bool check_src_eq(struct kunit *test, struct drm_plane_state 
>>> *plane_state,
>>> +static void check_src_eq(struct kunit *test, struct drm_plane_state 
>>> *plane_state,
>>>                      unsigned int src_x, unsigned int src_y,
>>>                      unsigned int src_w, unsigned int src_h)
>>>  {
>>>     struct drm_rect expected = DRM_RECT_INIT(src_x, src_y, src_w, src_h);
>>>  
>>> -   if (plane_state->src.x1 < 0) {
>>> -           kunit_err(test,
>>> -                     "src x coordinate %x should never be below 0, src: " 
>>> DRM_RECT_FP_FMT,
>>> -                     plane_state->src.x1, 
>>> DRM_RECT_FP_ARG(&plane_state->src));
>>> -           return false;
>>> -   }
>>> -   if (plane_state->src.y1 < 0) {
>>> -           kunit_err(test,
>>> -                     "src y coordinate %x should never be below 0, src: " 
>>> DRM_RECT_FP_FMT,
>>> -                     plane_state->src.y1, 
>>> DRM_RECT_FP_ARG(&plane_state->src));
>>> -           return false;
>>> -   }
>>> +   KUNIT_ASSERT_GE_MSG(test, plane_state->src.x1, 0,
>>> +                       "src x coordinate %x should never be below 0, src: 
>>> " DRM_RECT_FP_FMT,
>>> +                       plane_state->src.x1, 
>>> DRM_RECT_FP_ARG(&plane_state->src));
>>> +
>>> +   KUNIT_ASSERT_GE_MSG(test, plane_state->src.y1, 0,
>>> +                       "src y coordinate %x should never be below 0, src: 
>>> " DRM_RECT_FP_FMT,
>>> +                       plane_state->src.y1, 
>>> DRM_RECT_FP_ARG(&plane_state->src));
>>>  
>>>     if (plane_state->src.x1 != expected.x1 ||
>>>         plane_state->src.y1 != expected.y1 ||
>>>         drm_rect_width(&plane_state->src) != drm_rect_width(&expected) ||
>>>         drm_rect_height(&plane_state->src) != drm_rect_height(&expected)) {
>>> -           kunit_err(test, "src: " DRM_RECT_FP_FMT ", expected: " 
>>> DRM_RECT_FP_FMT,
>>> -                     DRM_RECT_FP_ARG(&plane_state->src), 
>>> DRM_RECT_FP_ARG(&expected));
>>> -
>>> -           return false;
>>> +           KUNIT_FAIL(test, "src: " DRM_RECT_FP_FMT ", expected: " 
>>> DRM_RECT_FP_FMT,
>>> +                      DRM_RECT_FP_ARG(&plane_state->src), 
>>> DRM_RECT_FP_ARG(&expected));
>>>     }
>>
>> I believe it would be more KUnit-like to write:
>>
>> KUNIT_ASSERT_EQ_MSG(test, plane_state->src.x1, expected.x1,
>>      "src: " DRM_RECT_FP_FMT ", expected: " DRM_RECT_FP_FMT,
>>      DRM_RECT_FP_ARG(&plane_state->src), DRM_RECT_FP_ARG(&expected));
>>
>> KUNIT_ASSERT_EQ_MSG(test, plane_state->src.y1, expected.y1,
>>      "src: " DRM_RECT_FP_FMT ", expected: " DRM_RECT_FP_FMT,
>>      DRM_RECT_FP_ARG(&plane_state->src), DRM_RECT_FP_ARG(&expected));
>>
>> KUNIT_ASSERT_EQ_MSG(test, drm_rect_width(&plane_state->src),
>>      drm_rect_width(&expected),
>>      "src: " DRM_RECT_FP_FMT ", expected: " DRM_RECT_FP_FMT,
>>      DRM_RECT_FP_ARG(&plane_state->src), DRM_RECT_FP_ARG(&expected));
>>
>> KUNIT_ASSERT_EQ_MSG(test, drm_rect_height(&plane_state->src),
>>      drm_rect_height(&expected),
>>      "src: " DRM_RECT_FP_FMT ", expected: " DRM_RECT_FP_FMT,
>>      DRM_RECT_FP_ARG(&plane_state->src), DRM_RECT_FP_ARG(&expected));
>>
>> And maybe explicit in the message which coordinate failed.
> 
> All of the components (width / height / x / y) used in the check are already
> printed in the proper format (DRM_RECT_FP_FMT is widthxheight+x1+y1).
> 
> The suggested approach uses an assert, which means that it will terminate the
> test on first failed check. If we use EXPECT instead, we will just print the
> same information multiple times.
> 
> We could replace it with multiple simple expects, and drop the DRM_RECT_FP_FMT
> usage, but that would give us less information and additionally, since the
> values here are in fixed-point format, it would just be confusing, since e.g.
> for width expect with "actual=1024.000000", "expected=2048.000000" we would 
> see
> 67108864 and 134217728 instead:
> 
> # drm_check_plane_state: ASSERTION FAILED at 
> drivers/gpu/drm/tests/drm_plane_helper_test.c:114
> Expected drm_rect_width(&plane_state->src) == drm_rect_width(&expected), but
> drm_rect_width(&plane_state->src) == 67108864
> drm_rect_width(&expected) == 134217728
> 
> Bottom line is - it really is a compound check, we're checking if struct
> drm_rect is equal to another struct drm_rect.

In this case, KUNIT_EXPECT_MEMEQ [1] would be really useful, but is not
merged yet. Maybe in the short term, you could use:

KUNIT_ASSERT_EQ_MSG(test, 0,
        memcmp(plane_state->src, expected, sizeof(struct drm_rect)),
        "src: " DRM_RECT_FP_FMT ", expected: "
        DRM_RECT_FP_FMT, DRM_RECT_FP_ARG(&plane_state->src),            
        DRM_RECT_FP_ARG(&expected));

Usually, this wouldn't be nice, as the assertion would just return the
memcmp error and wouldn't return the components. But, as there is a
proper way to print the components in the message, I believe this would
be a fine option.

[1]
https://lore.kernel.org/linux-kselftest/20220808125237.277126-1-mairaca...@riseup.net/

Best Regards,
- Maíra Canal

> The "KUnit-like" way would be to define a custom assert/expect, something like
> KUNIT_ASSERT_DRM_RECT_FP_EQ, but since this is just used in this single 
> check, I
> think it would be overengineering things slightly and would end up being less
> readable.
> 
> -Michał
> 
>>
>>> -
>>> -   return true;
>>>  }
>>>  
>>> -static void set_crtc(struct drm_plane_state *plane_state,
>>> -                int crtc_x, int crtc_y,
>>> -                unsigned int crtc_w, unsigned int crtc_h)
>>> -{
>>> -   plane_state->crtc_x = crtc_x;
>>> -   plane_state->crtc_y = crtc_y;
>>> -   plane_state->crtc_w = crtc_w;
>>> -   plane_state->crtc_h = crtc_h;
>>> -}
>>> -
>>> -static bool check_crtc_eq(struct kunit *test, struct drm_plane_state 
>>> *plane_state,
>>> +static void check_crtc_eq(struct kunit *test, struct drm_plane_state 
>>> *plane_state,
>>>                       int crtc_x, int crtc_y,
>>>                       unsigned int crtc_w, unsigned int crtc_h)
>>>  {
>>> @@ -74,171 +112,211 @@ static bool check_crtc_eq(struct kunit *test, struct 
>>> drm_plane_state *plane_stat
>>>         plane_state->dst.y1 != expected.y1 ||
>>>         drm_rect_width(&plane_state->dst) != drm_rect_width(&expected) ||
>>>         drm_rect_height(&plane_state->dst) != drm_rect_height(&expected)) {
>>> -           kunit_err(test, "dst: " DRM_RECT_FMT ", expected: " 
>>> DRM_RECT_FMT,
>>> -                     DRM_RECT_ARG(&plane_state->dst), 
>>> DRM_RECT_ARG(&expected));
>>> -
>>> -           return false;
>>> +           KUNIT_FAIL(test, "dst: " DRM_RECT_FMT ", expected: " 
>>> DRM_RECT_FMT,
>>> +                      DRM_RECT_ARG(&plane_state->dst), 
>>> DRM_RECT_ARG(&expected));
>>>     }
>>
>> Same goes here.
>>
>> Other than that,
>>
>> Reviewed-by: Maíra Canal <mairaca...@riseup.net>
>>
>> Best Regards,
>> - Maíra Canal
>>
>>> +}
>>> +
>>> +static void drm_check_plane_state(struct kunit *test)
>>> +{
>>> +   const struct drm_check_plane_state_test *params = test->param_value;
>>> +   struct drm_plane_state *plane_state = test->priv;
>>>  
>>> -   return true;
>>> +   KUNIT_ASSERT_EQ_MSG(test,
>>> +                       drm_atomic_helper_check_plane_state(plane_state, 
>>> &crtc_state,
>>> +                                                           
>>> params->min_scale,
>>> +                                                           
>>> params->max_scale,
>>> +                                                           
>>> params->can_position, false),
>>> +                       0, params->msg);
>>> +   KUNIT_EXPECT_TRUE(test, plane_state->visible);
>>> +   check_src_eq(test, plane_state, params->src_expected.x, 
>>> params->src_expected.y,
>>> +                params->src_expected.w, params->src_expected.h);
>>> +   check_crtc_eq(test, plane_state, params->crtc_expected.x, 
>>> params->crtc_expected.y,
>>> +                 params->crtc_expected.w, params->crtc_expected.h);
>>>  }
>>>  
>>> -static void igt_check_plane_state(struct kunit *test)
>>> +static void drm_check_plane_state_desc(const struct 
>>> drm_check_plane_state_test *t,
>>> +                                  char *desc)
>>>  {
>>> -   int ret;
>>> -
>>> -   static const struct drm_crtc_state crtc_state = {
>>> -           .crtc = ZERO_SIZE_PTR,
>>> -           .enable = true,
>>> -           .active = true,
>>> -           .mode = {
>>> -                   DRM_MODE("1024x768", 0, 65000, 1024, 1048, 1184, 1344, 
>>> 0, 768, 771,
>>> -                            777, 806, 0, DRM_MODE_FLAG_NHSYNC | 
>>> DRM_MODE_FLAG_NVSYNC)
>>> -           },
>>> -   };
>>> -   static struct drm_plane plane = {
>>> -           .dev = NULL
>>> -   };
>>> -   static struct drm_framebuffer fb = {
>>> -           .width = 2048,
>>> -           .height = 2048
>>> -   };
>>> -   static struct drm_plane_state plane_state = {
>>> -           .plane = &plane,
>>> -           .crtc = ZERO_SIZE_PTR,
>>> -           .fb = &fb,
>>> -           .rotation = DRM_MODE_ROTATE_0
>>> -   };
>>> -
>>> -   /* Simple clipping, no scaling. */
>>> -   set_src(&plane_state, 0, 0, fb.width << 16, fb.height << 16);
>>> -   set_crtc(&plane_state, 0, 0, fb.width, fb.height);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple clipping check should 
>>> pass\n");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1024 << 
>>> 16, 768 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 
>>> 768));
>>> -
>>> -   /* Rotated clipping + reflection, no scaling. */
>>> -   plane_state.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X;
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Rotated clipping check should 
>>> pass\n");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 768 << 
>>> 16, 1024 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 
>>> 768));
>>> -   plane_state.rotation = DRM_MODE_ROTATE_0;
>>> -
>>> -   /* Check whether positioning works correctly. */
>>> -   set_src(&plane_state, 0, 0, 1023 << 16, 767 << 16);
>>> -   set_crtc(&plane_state, 0, 0, 1023, 767);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_TRUE_MSG(test, ret,
>>> -                         "Should not be able to position on the crtc with 
>>> can_position=false\n");
>>> -
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             true, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple positioning should 
>>> work\n");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1023 << 
>>> 16, 767 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1023, 
>>> 767));
>>> -
>>> -   /* Simple scaling tests. */
>>> -   set_src(&plane_state, 0, 0, 512 << 16, 384 << 16);
>>> -   set_crtc(&plane_state, 0, 0, 1024, 768);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             0x8001,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_TRUE_MSG(test, ret, "Upscaling out of range should 
>>> fail.\n");
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             0x8000,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Upscaling exactly 2x should 
>>> work\n");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 512 << 
>>> 16, 384 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 
>>> 768));
>>> -
>>> -   set_src(&plane_state, 0, 0, 2048 << 16, 1536 << 16);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             0x1ffff, false, false);
>>> -   KUNIT_EXPECT_TRUE_MSG(test, ret, "Downscaling out of range should 
>>> fail.\n");
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             0x20000, false, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed with exact scaling 
>>> limit\n");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2048 << 
>>> 16, 1536 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 
>>> 768));
>>> -
>>> -   /* Testing rounding errors. */
>>> -   set_src(&plane_state, 0, 0, 0x40001, 0x40001);
>>> -   set_crtc(&plane_state, 1022, 766, 4, 4);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             0x10001,
>>> -                                             true, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to 
>>> exact multiple");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 
>>> 2 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 
>>> 2));
>>> -
>>> -   set_src(&plane_state, 0x20001, 0x20001, 0x4040001, 0x3040001);
>>> -   set_crtc(&plane_state, -2, -2, 1028, 772);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             0x10001,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to 
>>> exact multiple");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x40002, 
>>> 0x40002,
>>> -                                        1024 << 16, 768 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 
>>> 768));
>>> -
>>> -   set_src(&plane_state, 0, 0, 0x3ffff, 0x3ffff);
>>> -   set_crtc(&plane_state, 1022, 766, 4, 4);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             0xffff,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             true, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to 
>>> exact multiple");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   /* Should not be rounded to 0x20001, which would be upscaling. */
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 
>>> 2 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 
>>> 2));
>>> -
>>> -   set_src(&plane_state, 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff);
>>> -   set_crtc(&plane_state, -2, -2, 1028, 772);
>>> -   ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
>>> -                                             0xffff,
>>> -                                             DRM_PLANE_NO_SCALING,
>>> -                                             false, false);
>>> -   KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to 
>>> exact multiple");
>>> -   KUNIT_EXPECT_TRUE(test, plane_state.visible);
>>> -   KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x3fffe, 
>>> 0x3fffe,
>>> -                                        1024 << 16, 768 << 16));
>>> -   KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 
>>> 768));
>>> +   sprintf(desc, "%s", t->name);
>>>  }
>>>  
>>> +static const struct drm_check_plane_state_test 
>>> drm_check_plane_state_tests[] = {
>>> +   {
>>> +           .name = "clipping_simple",
>>> +           .msg = "Simple clipping check should pass",
>>> +           .src = { 0, 0,
>>> +                    2048 << 16,
>>> +                    2048 << 16 },
>>> +           .crtc = { 0, 0, 2048, 2048 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = false,
>>> +           .src_expected = { 0, 0, 1024 << 16, 768 << 16 },
>>> +           .crtc_expected = { 0, 0, 1024, 768 },
>>> +   },
>>> +   {
>>> +           .name = "clipping_rotate_reflect",
>>> +           .msg = "Rotated clipping check should pass",
>>> +           .src = { 0, 0,
>>> +                    2048 << 16,
>>> +                    2048 << 16 },
>>> +           .crtc = { 0, 0, 2048, 2048 },
>>> +           .rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = false,
>>> +           .src_expected = { 0, 0, 768 << 16, 1024 << 16 },
>>> +           .crtc_expected = { 0, 0, 1024, 768 },
>>> +   },
>>> +   {
>>> +           .name = "positioning_simple",
>>> +           .msg = "Simple positioning should work",
>>> +           .src = { 0, 0, 1023 << 16, 767 << 16 },
>>> +           .crtc = { 0, 0, 1023, 767 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = true,
>>> +           .src_expected = { 0, 0, 1023 << 16, 767 << 16 },
>>> +           .crtc_expected = { 0, 0, 1023, 767 },
>>> +   },
>>> +   {
>>> +           .name = "upscaling",
>>> +           .msg = "Upscaling exactly 2x should work",
>>> +           .src = { 0, 0, 512 << 16, 384 << 16 },
>>> +           .crtc = { 0, 0, 1024, 768 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = 0x8000,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = false,
>>> +           .src_expected = { 0, 0, 512 << 16, 384 << 16 },
>>> +           .crtc_expected = { 0, 0, 1024, 768 },
>>> +   },
>>> +   {
>>> +           .name = "downscaling",
>>> +           .msg = "Should succeed with exact scaling limit",
>>> +           .src = { 0, 0, 2048 << 16, 1536 << 16 },
>>> +           .crtc = { 0, 0, 1024, 768 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = 0x20000,
>>> +           .can_position = false,
>>> +           .src_expected = { 0, 0, 2048 << 16, 1536 << 16 },
>>> +           .crtc_expected = { 0, 0, 1024, 768 },
>>> +   },
>>> +   {
>>> +           .name = "rounding1",
>>> +           .msg = "Should succeed by clipping to exact multiple",
>>> +           .src = { 0, 0, 0x40001, 0x40001 },
>>> +           .crtc = { 1022, 766, 4, 4 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = 0x10001,
>>> +           .can_position = true,
>>> +           .src_expected = { 0, 0, 2 << 16, 2 << 16 },
>>> +           .crtc_expected = { 1022, 766, 2, 2 },
>>> +   },
>>> +   {
>>> +           .name = "rounding2",
>>> +           .msg = "Should succeed by clipping to exact multiple",
>>> +           .src = { 0x20001, 0x20001, 0x4040001, 0x3040001 },
>>> +           .crtc = { -2, -2, 1028, 772 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = 0x10001,
>>> +           .can_position = false,
>>> +           .src_expected = { 0x40002, 0x40002, 1024 << 16, 768 << 16 },
>>> +           .crtc_expected = { 0, 0, 1024, 768 },
>>> +   },
>>> +   {
>>> +           .name = "rounding3",
>>> +           .msg = "Should succeed by clipping to exact multiple",
>>> +           .src = { 0, 0, 0x3ffff, 0x3ffff },
>>> +           .crtc = { 1022, 766, 4, 4 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = 0xffff,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = true,
>>> +           /* Should not be rounded to 0x20001, which would be upscaling. 
>>> */
>>> +           .src_expected = { 0, 0, 2 << 16, 2 << 16 },
>>> +           .crtc_expected = { 1022, 766, 2, 2 },
>>> +   },
>>> +   {
>>> +           .name = "rounding4",
>>> +           .msg = "Should succeed by clipping to exact multiple",
>>> +           .src = { 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff },
>>> +           .crtc = { -2, -2, 1028, 772 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = 0xffff,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = false,
>>> +           .src_expected = { 0x3fffe, 0x3fffe, 1024 << 16, 768 << 16 },
>>> +           .crtc_expected = { 0, 0, 1024, 768 },
>>> +   },
>>> +};
>>> +
>>> +KUNIT_ARRAY_PARAM(drm_check_plane_state, drm_check_plane_state_tests, 
>>> drm_check_plane_state_desc);
>>> +
>>> +static void drm_check_invalid_plane_state(struct kunit *test)
>>> +{
>>> +   const struct drm_check_plane_state_test *params = test->param_value;
>>> +   struct drm_plane_state *plane_state = test->priv;
>>> +
>>> +   KUNIT_ASSERT_LT_MSG(test,
>>> +                       drm_atomic_helper_check_plane_state(plane_state, 
>>> &crtc_state,
>>> +                                                           
>>> params->min_scale,
>>> +                                                           
>>> params->max_scale,
>>> +                                                           
>>> params->can_position, false),
>>> +                       0, params->msg);
>>> +}
>>> +
>>> +static const struct drm_check_plane_state_test 
>>> drm_check_invalid_plane_state_tests[] = {
>>> +   {
>>> +           .name = "positioning_invalid",
>>> +           .msg = "Should not be able to position on the crtc with 
>>> can_position=false",
>>> +           .src = { 0, 0, 1023 << 16, 767 << 16 },
>>> +           .crtc = { 0, 0, 1023, 767 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = false,
>>> +   },
>>> +   {
>>> +           .name = "upscaling_invalid",
>>> +           .msg = "Upscaling out of range should fail",
>>> +           .src = { 0, 0, 512 << 16, 384 << 16 },
>>> +           .crtc = { 0, 0, 1024, 768 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = 0x8001,
>>> +           .max_scale = DRM_PLANE_NO_SCALING,
>>> +           .can_position = false,
>>> +   },
>>> +   {
>>> +           .name = "downscaling_invalid",
>>> +           .msg = "Downscaling out of range should fail",
>>> +           .src = { 0, 0, 2048 << 16, 1536 << 16 },
>>> +           .crtc = { 0, 0, 1024, 768 },
>>> +           .rotation = DRM_MODE_ROTATE_0,
>>> +           .min_scale = DRM_PLANE_NO_SCALING,
>>> +           .max_scale = 0x1ffff,
>>> +           .can_position = false,
>>> +   },
>>> +};
>>> +
>>> +KUNIT_ARRAY_PARAM(drm_check_invalid_plane_state, 
>>> drm_check_invalid_plane_state_tests,
>>> +             drm_check_plane_state_desc);
>>> +
>>>  static struct kunit_case drm_plane_helper_test[] = {
>>> -   KUNIT_CASE(igt_check_plane_state),
>>> +   KUNIT_CASE_PARAM(drm_check_plane_state, 
>>> drm_check_plane_state_gen_params),
>>> +   KUNIT_CASE_PARAM(drm_check_invalid_plane_state, 
>>> drm_check_invalid_plane_state_gen_params),
>>>     {}
>>>  };
>>>  
>>>  static struct kunit_suite drm_plane_helper_test_suite = {
>>>     .name = "drm_plane_helper",
>>> +   .init = drm_plane_helper_init,
>>>     .test_cases = drm_plane_helper_test,
>>>  };
>>>  

Reply via email to