From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 27F2D6E8BA for ; Thu, 29 Oct 2020 12:43:08 +0000 (UTC) From: "Kahola, Mika" Date: Thu, 29 Oct 2020 12:43:06 +0000 Message-ID: References: <20201026130811.11136-1-juhapekka.heikkila@gmail.com> In-Reply-To: <20201026130811.11136-1-juhapekka.heikkila@gmail.com> Content-Language: en-US MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_plane: fix pixel format (-clamping) tests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Juha-Pekka Heikkila , "igt-dev@lists.freedesktop.org" List-ID: > -----Original Message----- > From: igt-dev On Behalf Of Juha- > Pekka Heikkila > Sent: Monday, October 26, 2020 3:08 PM > To: igt-dev@lists.freedesktop.org > Subject: [igt-dev] [PATCH i-g-t] tests/kms_plane: fix pixel format (-clamping) > tests > > cdclk caused modesets mid test will mess up crc sequence where two > consecutive frames will get same crc. If there come modeset restart round > for given set as modeset will anyway happen only on first color. > > Signed-off-by: Juha-Pekka Heikkila > --- > tests/kms_plane.c | 28 ++++++++++++++++++++++------ > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/tests/kms_plane.c b/tests/kms_plane.c index > e42c71cd9..4ce859fba 100644 > --- a/tests/kms_plane.c > +++ b/tests/kms_plane.c > @@ -560,9 +560,11 @@ static void capture_format_crcs(data_t *data, enum > pipe pipe, > struct drm_event_vblank ev; > int i; > > +restart_round: > for (i = 0; i < data->num_colors; i++) { > const color_t *c = &data->colors[i]; > struct igt_fb old_fb = *fb; > + int ret; > > prepare_format_color(data, pipe, plane, format, modifier, > width, height, encoding, range, c, fb); @@ > -593,10 +595,22 @@ static void capture_format_crcs(data_t *data, enum > pipe pipe, > * to be prepared in parallel while the current fb > * awaits to be latched. > */ > - igt_display_commit_atomic(&data->display, > - > DRM_MODE_ATOMIC_ALLOW_MODESET | > - > DRM_MODE_ATOMIC_NONBLOCK | > - > DRM_MODE_PAGE_FLIP_EVENT, NULL); > + ret = igt_display_try_commit_atomic(&data->display, > + > DRM_MODE_ATOMIC_NONBLOCK | > + > DRM_MODE_PAGE_FLIP_EVENT, NULL); > + if (ret) { > + /* > + * there was needed modeset for pixel > format. > + * modeset here happen only on first color of > + * given set so restart round as modeset will > + * mess up crc frame sequence. > + */ > + igt_display_commit_atomic(&data->display, > + > DRM_MODE_ATOMIC_ALLOW_MODESET, > + NULL); > + igt_remove_fb(data->drm_fd, &old_fb); > + goto restart_round; > + } I tried to figure out the patch and it seems that we need to do modeset only once, right? However, there is a theoretical chance (probability maybe next to nothing) that we keep looping on restart_found. Therefore, I was thinking that we should limit the restarts to 1 or to some finite number? Cheers, Mika > } else { > /* > * Last moment to grab the previous crc @@ -815,8 > +829,10 @@ static bool test_format_plane(data_t *data, enum pipe pipe, > > igt_plane_set_fb(plane, &test_fb); > > - ret = igt_display_try_commit_atomic(&data->display, > DRM_MODE_ATOMIC_TEST_ONLY, NULL); > - > + ret = igt_display_try_commit_atomic(&data->display, > + > DRM_MODE_ATOMIC_TEST_ONLY | > + > DRM_MODE_ATOMIC_ALLOW_MODESET, > + NULL); > if (!ret) { > width = test_fb.width; > height = test_fb.height; > -- > 2.28.0 > > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev