Re: [PATCH 1/6] drm/format-helper: Test default pitch fallback

[Maira Canal]

Hi Arthur,

Just nitpicking, but...

On 7/21/23 15:23, Arthur Grillo wrote:

Test the default pitch fallback when NULL is passed as the dst_pitch on
the conversion procedures.

Signed-off-by: Arthur Grillo <arthurgri...@riseup.net>
---
  .../gpu/drm/tests/drm_format_helper_test.c    | 132 ++++++++++++------
  1 file changed, 87 insertions(+), 45 deletions(-)

[...]

@@ -530,7 +532,10 @@ static void drm_test_fb_xrgb8888_to_gray8(struct kunit 
*test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_gray8(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_gray8(&dst, NULL, &src, &fb, &params->clip);
+       else
+               drm_fb_xrgb8888_to_gray8(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);

Couldn't we do something like:

unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
        ? NULL : &result->dst_pitch;

[...]

drm_fb_xrgb8888_to_gray8(&dst, dst_pitch, &src, &fb, &params->clip);

I believe the code would be cleaner.

Best Regards,
- Maíra

        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }

@@ -560,7 +565,10 @@ static void drm_test_fb_xrgb8888_to_rgb332(struct kunit *test)

        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_rgb332(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_rgb332(&dst, NULL, &src, &fb, &params->clip);
+       else
+               drm_fb_xrgb8888_to_rgb332(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }

@@ -590,12 +598,19 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)

        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip, false);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_rgb565(&dst, NULL, &src, &fb, &params->clip, 
false);
+       else
+               drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, 
&params->clip,
+                                         false);
        buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / 
sizeof(__le16));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);

buf = dst.vaddr; /* restore original value of buf */

-       drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, 
&params->clip, true);
+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_rgb565(&dst, NULL, &src, &fb, &params->clip, 
true);
+       else
+               drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, 
&params->clip, true);
        buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / 
sizeof(__le16));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected_swab, dst_size);
  }
@@ -626,7 +641,10 @@ static void drm_test_fb_xrgb8888_to_xrgb1555(struct kunit 
*test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_xrgb1555(&dst, NULL, &src, &fb, 
&params->clip);
+       else
+               drm_fb_xrgb8888_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
        buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / 
sizeof(__le16));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }
@@ -657,7 +675,10 @@ static void drm_test_fb_xrgb8888_to_argb1555(struct kunit 
*test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_argb1555(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_argb1555(&dst, NULL, &src, &fb, 
&params->clip);
+       else
+               drm_fb_xrgb8888_to_argb1555(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
        buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / 
sizeof(__le16));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }
@@ -688,7 +709,10 @@ static void drm_test_fb_xrgb8888_to_rgba5551(struct kunit 
*test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
+       else
+               drm_fb_xrgb8888_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
        buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / 
sizeof(__le16));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }
@@ -723,7 +747,11 @@ static void drm_test_fb_xrgb8888_to_rgb888(struct kunit 
*test)
         * RGB888 expected results are already in little-endian
         * order, so there's no need to convert the test output.
         */
-       drm_fb_xrgb8888_to_rgb888(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_rgb888(&dst, NULL, &src, &fb, &params->clip);
+       else
+               drm_fb_xrgb8888_to_rgb888(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
+
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }

@@ -753,7 +781,11 @@ static void drm_test_fb_xrgb8888_to_argb8888(struct kunit *test)

        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_argb8888(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_argb8888(&dst, NULL, &src, &fb, 
&params->clip);
+       else
+               drm_fb_xrgb8888_to_argb8888(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
+
        buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size / 
sizeof(u32));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }
@@ -784,7 +816,10 @@ static void drm_test_fb_xrgb8888_to_xrgb2101010(struct 
kunit *test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_xrgb2101010(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_xrgb2101010(&dst, NULL, &src, &fb, 
&params->clip);
+       else
+               drm_fb_xrgb8888_to_xrgb2101010(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
        buf = le32buf_to_cpu(test, buf, dst_size / sizeof(u32));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }
@@ -815,7 +850,11 @@ static void drm_test_fb_xrgb8888_to_argb2101010(struct 
kunit *test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_argb2101010(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_argb2101010(&dst, NULL, &src, &fb, 
&params->clip);
+       else
+               drm_fb_xrgb8888_to_argb2101010(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
+
        buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size / 
sizeof(u32));
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }
@@ -846,7 +885,10 @@ static void drm_test_fb_xrgb8888_to_mono(struct kunit 
*test)
        KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
        iosys_map_set_vaddr(&src, xrgb8888);

- drm_fb_xrgb8888_to_mono(&dst, &result->dst_pitch, &src, &fb, &params->clip);

+       if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+               drm_fb_xrgb8888_to_mono(&dst, NULL, &src, &fb, &params->clip);
+       else
+               drm_fb_xrgb8888_to_mono(&dst, &result->dst_pitch, &src, &fb, 
&params->clip);
        KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
  }