From: Karthik B S <karthik.b.s@intel.com>
To: Jason-JH Lin <jason-jh.lin@mediatek.com>,
<igt-dev@lists.freedesktop.org>,
Swati Sharma <swati2.sharma@intel.com>,
Kamil Konieczny <kamil.konieczny@linux.intel.com>,
Ville Syrjala <ville.syrjala@linux.intel.com>,
Fei Shao <fshao@chromium.org>,
"Juha-Pekka Heikkila" <juhapekka.heikkila@gmail.com>
Cc: Jani <jani.nikula@intel.com>,
Paul-PL Chen <paul-pl.chen@mediatek.com>,
Lancelot Wu <Lancelot.Wu@mediatek.com>,
Manasi Navare <navaremanasi@google.com>,
Gil Dekel <gildekel@google.com>, Yacoub <markyacoub@chromium.org>,
<Project_Global_Chrome_Upstream_Group@mediatek.com>
Subject: Re: [PATCH i-g-t v2] tests/kms_plane: Remove redundant CRC frame sequence check
Date: Thu, 13 Aug 2026 09:07:24 +0530 [thread overview]
Message-ID: <7244584b-4bed-4756-bdae-e06b0054d2b7@intel.com> (raw)
In-Reply-To: <20260811161417.716771-1-jason-jh.lin@mediatek.com>
Hi Jason-JH,
On 8/11/2026 9:44 PM, Jason-JH Lin wrote:
> The capture_crc() function validated that the CRC frame sequence
> returned by igt_pipe_crc_get_for_frame() matches the expected value.
> However, igt_pipe_crc_get_for_frame() already guarantees
> crc->frame >= expected via its internal loop:
>
> do {
> read_one_crc(pipe_crc, crc);
> } while (igt_vblank_before(crc->frame, vblank));
This isn't fully true IMHO. The capture CRC function actually ensured
the exact match of frame sequence and with this patch we're just
guaranteeing '>='.
So we need more context here from:
https://patchwork.freedesktop.org/series/168037/
>
> The additional check in capture_crc() is therefore redundant.
> Remove it and rely on the library's existing guarantee.
>
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
> tests/kms_plane.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 12dfbfe1d82b..fe8ee2ab26ab 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -765,11 +765,6 @@ static int num_unique_crcs(const igt_crc_t crc[], int num_crc)
> static void capture_crc(data_t *data, unsigned int vblank, igt_crc_t *crc)
> {
> igt_pipe_crc_get_for_frame(data->drm_fd, data->pipe_crc, vblank, crc);
Also if this is only igt_pipe_crc_get_for_frame now, ideally we can just
remove this function itself and call the helper directly. But before
doing that, as the existing assert was added by a patch from Ville and
rb'ed by JP, I'll request an ack from them or if they have any inputs on
this.
Regards,
Karthik.B.S
> -
> - igt_fail_on_f(!igt_skip_crc_compare && !igt_run_in_simulation() &&
> - crc->has_valid_frame && crc->frame != vblank,
> - "Got CRC for the wrong frame (got %u, expected %u). CRC buffer overflow?\n",
> - crc->frame, vblank);
> }
>
> static void capture_format_crcs_single(data_t *data, igt_crtc_t *crtc,
next prev parent reply other threads:[~2026-08-13 3:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-11 16:14 [PATCH i-g-t v2] tests/kms_plane: Remove redundant CRC frame sequence check Jason-JH Lin
2026-08-11 20:31 ` ✓ i915.CI.BAT: success for " Patchwork
2026-08-11 20:38 ` ✓ Xe.CI.BAT: " Patchwork
2026-08-11 22:33 ` ✓ Xe.CI.FULL: " Patchwork
2026-08-12 19:28 ` [PATCH i-g-t v2] " Manasi Navare
2026-08-13 3:37 ` Karthik B S [this message]
2026-08-13 5:36 ` Jason-JH Lin (林睿祥)
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=7244584b-4bed-4756-bdae-e06b0054d2b7@intel.com \
--to=karthik.b.s@intel.com \
--cc=Lancelot.Wu@mediatek.com \
--cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
--cc=fshao@chromium.org \
--cc=gildekel@google.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=jason-jh.lin@mediatek.com \
--cc=juhapekka.heikkila@gmail.com \
--cc=kamil.konieczny@linux.intel.com \
--cc=markyacoub@chromium.org \
--cc=navaremanasi@google.com \
--cc=paul-pl.chen@mediatek.com \
--cc=swati2.sharma@intel.com \
--cc=ville.syrjala@linux.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.