From: "Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>
To: Swati Sharma <swati2.sharma@intel.com>, <igt-dev@lists.freedesktop.org>
Subject: Re: [igt-dev] [PATCH i-g-t 1/6] tests/kms_atomic: cosmetic changes
Date: Tue, 26 Sep 2023 22:07:45 +0530 [thread overview]
Message-ID: <4414526a-b00e-2c79-b523-5427451fe4ac@intel.com> (raw)
In-Reply-To: <20230918075411.247122-2-swati2.sharma@intel.com>
On Mon-18-09-2023 01:24 pm, Swati Sharma wrote:
> Removed unnecessary newlines. Corrected multi-line comments.
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
> tests/kms_atomic.c | 101 +++++++++++++++++++++++++--------------------
> 1 file changed, 57 insertions(+), 44 deletions(-)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index d1511716a..951014685 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -74,7 +74,6 @@ static inline int damage_rect_height(struct drm_mode_rect *r)
> return r->y2 - r->y1;
> }
>
> -
> static bool plane_filter(enum igt_atomic_plane_properties prop)
> {
> if ((1 << prop) & IGT_PLANE_COORD_CHANGED_MASK)
> @@ -121,8 +120,10 @@ static void plane_check_current_state(igt_plane_t *plane, const uint64_t *values
>
> plane_get_current_state(plane, current_values);
>
> - /* Legacy cursor ioctls create their own, unknowable, internal
> - * framebuffer which we can't reason about. */
> + /*
> + * Legacy cursor ioctls create their own, unknowable, internal
> + * framebuffer which we can't reason about.
> + */
> if (relax & PLANE_RELAX_FB)
> current_values[IGT_PLANE_FB_ID] = values[IGT_PLANE_FB_ID];
>
> @@ -188,7 +189,6 @@ static void crtc_check_current_state(igt_pipe_t *pipe,
> if (pipe_values[IGT_CRTC_MODE_ID]) {
> mode_prop = drmModeGetPropertyBlob(pipe->display->drm_fd,
> pipe_values[IGT_CRTC_MODE_ID]);
> -
> igt_assert(mode_prop);
>
> igt_assert_eq(mode_prop->length,
> @@ -220,9 +220,11 @@ static void crtc_check_current_state(igt_pipe_t *pipe,
>
> crtc_get_current_state(pipe, current_pipe_values);
>
> - /* Optionally relax the check for MODE_ID: using the legacy SetCrtc
> + /*
> + * Optionally relax the check for MODE_ID: using the legacy SetCrtc
> * API can potentially change MODE_ID even if the mode itself remains
> - * unchanged. */
> + * unchanged.
> + */
> if (relax & CRTC_RELAX_MODE && mode && current_pipe_values[IGT_CRTC_MODE_ID] &&
> current_pipe_values[IGT_CRTC_MODE_ID] != pipe_values[IGT_CRTC_MODE_ID]) {
> drmModePropertyBlobRes *cur_prop =
> @@ -475,8 +477,8 @@ plane_immutable_zpos(igt_display_t *display, igt_pipe_t *pipe,
> igt_assert(fb_id_upper);
>
> /*
> - * checking only pairs of plane in increasing fashion
> - * to avoid combinatorial explosion
> + * Checking only pairs of plane in increasing fashion
> + * to avoid combinatorial explosion.
> */
> for (int i = 0; i < n_planes - 1; i++) {
> igt_plane_t *plane_lower, *plane_upper;
> @@ -555,8 +557,10 @@ static void plane_overlay(igt_pipe_t *pipe, igt_output_t *output, igt_plane_t *p
> igt_plane_set_fb(plane, &fb);
> igt_plane_set_position(plane, w/2, h/2);
>
> - /* Enable the overlay plane using the atomic API, and double-check
> - * state is what we think it should be. */
> + /*
> + * Enable the overlay plane using the atomic API, and double-check
> + * state is what we think it should be.
> + */
> plane_commit(plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>
> /* Disable the plane and check the state matches the old. */
> @@ -564,14 +568,18 @@ static void plane_overlay(igt_pipe_t *pipe, igt_output_t *output, igt_plane_t *p
> igt_plane_set_position(plane, 0, 0);
> plane_commit(plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>
> - /* Re-enable the plane through the legacy plane API, and verify through
> - * atomic. */
> + /*
> + * Re-enable the plane through the legacy plane API, and verify through
> + * atomic.
> + */
> igt_plane_set_fb(plane, &fb);
> igt_plane_set_position(plane, w/2, h/2);
> plane_commit(plane, COMMIT_LEGACY, ATOMIC_RELAX_NONE);
>
> - /* Restore the plane to its original settings through the legacy plane
> - * API, and verify through atomic. */
> + /*
> + * Restore the plane to its original settings through the legacy plane
> + * API, and verify through atomic.
> + */
> igt_plane_set_fb(plane, NULL);
> igt_plane_set_position(plane, 0, 0);
> plane_commit(plane, COMMIT_LEGACY, ATOMIC_RELAX_NONE);
> @@ -588,8 +596,10 @@ static void plane_primary(igt_pipe_t *pipe, igt_plane_t *plane, struct igt_fb *f
> fb->drm_format, DRM_FORMAT_MOD_LINEAR,
> 0.2, 0.2, 0.2, &fb2);
>
> - /* Flip the primary plane using the atomic API, and double-check
> - * state is what we think it should be. */
> + /*
> + * Flip the primary plane using the atomic API, and double-check
> + * state is what we think it should be.
> + */
> igt_plane_set_fb(plane, &fb2);
> crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>
> @@ -597,18 +607,24 @@ static void plane_primary(igt_pipe_t *pipe, igt_plane_t *plane, struct igt_fb *f
> igt_plane_set_fb(plane, fb);
> crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>
> - /* Set the plane through the legacy CRTC/primary-plane API, and
> - * verify through atomic. */
> + /*
> + * Set the plane through the legacy CRTC/primary-plane API, and
> + * verify through atomic.
> + */
> igt_plane_set_fb(plane, &fb2);
> crtc_commit(pipe, plane, COMMIT_LEGACY, CRTC_RELAX_MODE);
>
> - /* Restore the plane to its original settings through the legacy CRTC
> - * API, and verify through atomic. */
> + /*
> + * Restore the plane to its original settings through the legacy CRTC
> + * API, and verify through atomic.
> + */
> igt_plane_set_fb(plane, fb);
> crtc_commit(pipe, plane, COMMIT_LEGACY, CRTC_RELAX_MODE);
>
> - /* Set the plane through the universal setplane API, and
> - * verify through atomic. */
> + /*
> + * Set the plane through the universal setplane API, and
> + * verify through atomic.
> + */
> igt_plane_set_fb(plane, &fb2);
> plane_commit(plane, COMMIT_UNIVERSAL, ATOMIC_RELAX_NONE);
> }
> @@ -623,7 +639,8 @@ static void plane_primary(igt_pipe_t *pipe, igt_plane_t *plane, struct igt_fb *f
> * Test category: functionality test
> */
>
> -/* test to ensure that DRM_MODE_ATOMIC_TEST_ONLY really only touches the
> +/*
> + * Test to ensure that DRM_MODE_ATOMIC_TEST_ONLY really only touches the
> * free-standing state objects and nothing else.
> */
> static void test_only(igt_pipe_t *pipe_obj,
> @@ -703,8 +720,10 @@ static void plane_cursor(igt_pipe_t *pipe_obj,
> DRM_FORMAT_MOD_LINEAR,
> 0.0, 0.0, 0.0, &fb);
>
> - /* Flip the cursor plane using the atomic API, and double-check
> - * state is what we think it should be. */
> + /*
> + * Flip the cursor plane using the atomic API, and double-check
> + * state is what we think it should be.
> + */
> igt_plane_set_fb(cursor, &fb);
> igt_plane_set_position(cursor, x, y);
> plane_commit(cursor, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
> @@ -714,8 +733,10 @@ static void plane_cursor(igt_pipe_t *pipe_obj,
> igt_plane_set_position(cursor, 0, 0);
> plane_commit(cursor, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>
> - /* Re-enable the plane through the legacy cursor API, and verify
> - * through atomic. */
> + /*
> + * Re-enable the plane through the legacy cursor API, and verify
> + * through atomic.
> + */
> igt_plane_set_fb(cursor, &fb);
> igt_plane_set_position(cursor, x, y);
> plane_commit(cursor, COMMIT_LEGACY, PLANE_RELAX_FB);
> @@ -724,8 +745,10 @@ static void plane_cursor(igt_pipe_t *pipe_obj,
> igt_plane_set_position(cursor, x - 16, y - 16);
> plane_commit(cursor, COMMIT_LEGACY, PLANE_RELAX_FB);
>
> - /* Restore the plane to its original settings through the legacy cursor
> - * API, and verify through atomic. */
> + /*
> + * Restore the plane to its original settings through the legacy cursor
> + * API, and verify through atomic.
> + */
> igt_plane_set_fb(cursor, NULL);
> igt_plane_set_position(cursor, 0, 0);
> plane_commit(cursor, COMMIT_LEGACY, ATOMIC_RELAX_NONE);
> @@ -1004,8 +1027,10 @@ static void crtc_invalid_params_fence(igt_pipe_t *pipe,
> * Test category: functionality test
> */
>
> -/* Abuse the atomic ioctl directly in order to test various invalid conditions,
> - * which the libdrm wrapper won't allow us to create. */
> +/*
> + * Abuse the atomic ioctl directly in order to test various invalid conditions,
> + * which the libdrm wrapper won't allow us to create.
> + */
> static void atomic_invalid_params(igt_pipe_t *pipe,
> igt_plane_t *plane,
> igt_output_t *output,
> @@ -1088,7 +1113,6 @@ static void atomic_invalid_params(igt_pipe_t *pipe,
> do_ioctl_err(display->drm_fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
>
> /* Valid property, valid value. */
> -
> for (i = 0; i < ARRAY_SIZE(props_raw); i++) {
> props_raw[i] = pipe->props[IGT_CRTC_MODE_ID];
> values_raw[i] = pipe->values[IGT_CRTC_MODE_ID];
> @@ -1227,7 +1251,7 @@ static void atomic_plane_damage(igt_pipe_t *pipe, igt_plane_t *plane, struct igt
> * NOTE: This will result in no update on plane as damage is outside, so
> * will see no change on the screen.
> */
> - /* Reszie fb_1 to be bigger than plane */
> + /* Resize fb_1 to be bigger than plane */
> igt_remove_fb(pipe->display->drm_fd, &fb_1);
> igt_create_color_fb(pipe->display->drm_fd, fb->width * 2, fb->height,
> fb->drm_format, DRM_FORMAT_MOD_LINEAR, 0.2, 0.2, 0.2,
> @@ -1390,9 +1414,7 @@ igt_main
>
> igt_fixture {
> display.drm_fd = drm_open_driver_master(DRIVER_ANY);
> -
> kmstest_set_vt_graphics_mode();
> -
> igt_display_require(&display, display.drm_fd);
> igt_require(display.is_atomic);
> igt_display_require_output(&display);
> @@ -1424,7 +1446,6 @@ igt_main
> igt_subtest("plane-overlay-legacy") {
> igt_plane_t *overlay =
> igt_pipe_get_plane_type(pipe_obj, DRM_PLANE_TYPE_OVERLAY);
> -
> igt_require(overlay);
>
> atomic_setup(&display, pipe, output, primary, &fb);
> @@ -1435,7 +1456,6 @@ igt_main
> "the legacy and atomic interfaces.");
> igt_subtest("plane-primary-legacy") {
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> plane_primary(pipe_obj, primary, &fb);
> }
>
> @@ -1491,7 +1511,6 @@ igt_main
> igt_subtest("plane-cursor-legacy") {
> igt_plane_t *cursor =
> igt_pipe_get_plane_type(pipe_obj, DRM_PLANE_TYPE_CURSOR);
> -
> igt_require(cursor);
>
> atomic_setup(&display, pipe, output, primary, &fb);
> @@ -1501,28 +1520,24 @@ igt_main
> igt_describe("Test error handling when invalid plane parameters are passed");
> igt_subtest("plane-invalid-params") {
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> plane_invalid_params(pipe_obj, output, primary, &fb);
> }
>
> igt_describe("Test error handling when invalid plane fence parameters are passed");
> igt_subtest("plane-invalid-params-fence") {
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> plane_invalid_params_fence(pipe_obj, output, primary);
> }
>
> igt_describe("Test error handling when invalid crtc parameters are passed");
> igt_subtest("crtc-invalid-params") {
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> crtc_invalid_params(pipe_obj, output, primary, &fb);
> }
>
> igt_describe("Test error handling when invalid crtc fence parameters are passed");
> igt_subtest("crtc-invalid-params-fence") {
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> crtc_invalid_params_fence(pipe_obj, output, primary, &fb);
> }
>
> @@ -1531,7 +1546,6 @@ igt_main
> "allow us to create.");
> igt_subtest("atomic-invalid-params") {
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> atomic_invalid_params(pipe_obj, primary, output, &fb);
> }
>
> @@ -1540,7 +1554,6 @@ igt_main
> igt_require(igt_plane_has_prop(primary, IGT_PLANE_FB_DAMAGE_CLIPS));
>
> atomic_setup(&display, pipe, output, primary, &fb);
> -
> atomic_plane_damage(pipe_obj, primary, &fb);
> }
>
next prev parent reply other threads:[~2023-09-26 16:38 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-18 7:54 [igt-dev] [PATCH i-g-t 0/6] tests/kms_atomic: Test cleanup and dynamic conversion Swati Sharma
2023-09-18 7:54 ` [igt-dev] [PATCH i-g-t 1/6] tests/kms_atomic: cosmetic changes Swati Sharma
2023-09-26 16:37 ` Modem, Bhanuprakash [this message]
2023-09-18 7:54 ` [igt-dev] [PATCH i-g-t 2/6] tests/kms_atomic: rename subtest Swati Sharma
2023-09-26 16:38 ` Modem, Bhanuprakash
2023-09-18 7:54 ` [igt-dev] [PATCH i-g-t 3/6] tests/kms_atomic: test cleanup Swati Sharma
2023-09-27 3:32 ` Karthik B S
2023-10-10 5:47 ` Sharma, Swati2
2023-09-18 7:54 ` [igt-dev] [PATCH i-g-t 4/6] tests/kms_atomic: convert subtests to dynamic subtests Swati Sharma
2023-09-27 3:39 ` Karthik B S
2023-10-10 5:54 ` Sharma, Swati2
2023-09-18 7:54 ` [igt-dev] [PATCH i-g-t 5/6] tests/kms_atomic: check validity of pipe-output combo Swati Sharma
2023-09-18 7:54 ` [igt-dev] [PATCH i-g-t 6/6] tests/kms_atomic: add flexibility to run tests on all pipes Swati Sharma
2023-09-18 11:54 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_atomic: Test cleanup and dynamic conversion Patchwork
2023-09-18 11:57 ` [igt-dev] ✗ CI.xeBAT: " Patchwork
2023-09-21 6:50 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2023-09-21 16:39 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2023-09-18 5:13 [igt-dev] [PATCH i-g-t 0/6] " Swati Sharma
2023-09-18 5:13 ` [igt-dev] [PATCH i-g-t 1/6] tests/kms_atomic: cosmetic changes Swati Sharma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4414526a-b00e-2c79-b523-5427451fe4ac@intel.com \
--to=bhanuprakash.modem@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=swati2.sharma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox