On Tue, Sep 29, 2020 at 12:13:23PM -0400, Lyude Paul wrote:
> On Mon, 2020-09-28 at 17:36 -0400, Jeremy Cline wrote:
> > Hi,
> >
> > On Tue, Aug 18, 2020 at 05:00:51PM -0400, Lyude wrote:
> > > From: Lyude Paul <lyude@xxxxxxxxxx>
> > >
> > > We're finally getting CRC support in nouveau, so let's start testing
> > > this in igt as well! While the normal CRC capture tests are nice,
> > > there's a number of Nvidia-specific hardware characteristics that we
> > > need to test as well.
> > >
> > > The most important one is known as a "notifier context flip". Basically,
> > > Nvidia GPUs store generated CRCs in an allocated memory region, referred
> > > to as the notifier context, that the driver programs itself. Strangely,
> > > this region can only hold a limited number of CRC entries, and once it
> > > runs out of available entries the hardware simply sets an overrun bit
> > > and stops writing any new CRC entries.
> > >
> > > Since igt-gpu-tools doesn't really have an expectation of only being
> > > able to grab a limited number of CRCs, we work around this in nouveau by
> > > allocating two separate CRC notifier regions each time we start
> > > capturing CRCs, and then flip between them whenever we get close to
> > > filling our currently programmed notifier context. While this keeps the
> > > number of CRC entries we lose to an absolute minimum, we are guaranteed
> > > to lose exactly one CRC entry between context flips. Thus, we add some
> > > tests to ensure that nouveau handles these flips correctly
> > > (pipe-[A-F]-ctx-flip-detection), and that igt itself is also able to
> > > handle them correctly (pipe-[A-F]-ctx-flip-skip-current-frame). Since
> > > these tests use a debugfs interface to manually control the notifier
> > > context flip threshold, we also add one test to ensure that any flip
> > > thresholds we set are cleared after a single CRC capture
> > > (ctx-flip-threshold-reset-after-capture).
> > >
> > > In addition, we also add some simple tests to test Nvidia-specific CRC
> > > sources.
> > >
> > > Changes since v3:
> > > * Update .gitlab-ci.yml to make nouveau exempt from the test-list-diff
> > > test, since all the cool kids are doing it and we don't care about
> > > autotools for nouveau
> > > Changes since v2:
> > > * Fix missing include in tests/nouveau_crc.c, this should fix builds for
> > > aarch64
> > > Changes since v1:
> > > * Fix copyright year in nouveau_crc.c
> > >
> > > Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx>
> > > ---
> > > .gitlab-ci.yml | 2 +-
> > > lib/drmtest.c | 10 ++
> > > lib/drmtest.h | 2 +
> > > tests/meson.build | 1 +
> > > tests/nouveau_crc.c | 397 ++++++++++++++++++++++++++++++++++++++++++++
> > > 5 files changed, 411 insertions(+), 1 deletion(-)
> > > create mode 100644 tests/nouveau_crc.c
> > >
> > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > index d7fdbfde..e226d9d7 100644
> > > --- a/.gitlab-ci.yml
> > > +++ b/.gitlab-ci.yml
> > > @@ -241,7 +241,7 @@ test:test-list-diff:
> > > - build:tests-debian-autotools
> > > - build:tests-debian-meson
> > > stage: test
> > > - script: diff <(sed "s/ /\n/g" meson-test-list.txt| grep -v
> > > 'vc4\|v3d\|panfrost' | sort) <(sed "s/ /\n/g" autotools-test-list.txt |
> > > sort)
> > > + script: diff <(sed "s/ /\n/g" meson-test-list.txt| grep -v
> > > 'vc4\|v3d\|panfrost\|nouveau' | sort) <(sed "s/ /\n/g" autotools-test-
> > > list.txt | sort)
> > >
> > > test:list-undocumented-tests:
> > > dependencies:
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index c732d1dd..447f5435 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -114,6 +114,11 @@ bool is_i915_device(int fd)
> > > return __is_device(fd, "i915");
> > > }
> > >
> > > +bool is_nouveau_device(int fd)
> > > +{
> > > + return __is_device(fd, "nouveau");
> > > +}
> > > +
> > > bool is_vc4_device(int fd)
> > > {
> > > return __is_device(fd, "vc4");
> > > @@ -622,6 +627,11 @@ void igt_require_intel(int fd)
> > > igt_require(is_i915_device(fd));
> > > }
> > >
> > > +void igt_require_nouveau(int fd)
> > > +{
> > > + igt_require(is_nouveau_device(fd));
> > > +}
> > > +
> > > void igt_require_vc4(int fd)
> > > {
> > > igt_require(is_vc4_device(fd));
> > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > index c56bfafa..dd4cd384 100644
> > > --- a/lib/drmtest.h
> > > +++ b/lib/drmtest.h
> > > @@ -96,10 +96,12 @@ int __drm_open_driver_render(int chipset);
> > >
> > > void igt_require_amdgpu(int fd);
> > > void igt_require_intel(int fd);
> > > +void igt_require_nouveau(int fd);
> > > void igt_require_vc4(int fd);
> > >
> > > bool is_amdgpu_device(int fd);
> > > bool is_i915_device(int fd);
> > > +bool is_nouveau_device(int fd);
> > > bool is_vc4_device(int fd);
> > >
> > > /**
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index 684de043..92647991 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -74,6 +74,7 @@ test_progs = [
> > > 'kms_vblank',
> > > 'kms_vrr',
> > > 'meta_test',
> > > + 'nouveau_crc',
> > > 'panfrost_get_param',
> > > 'panfrost_gem_new',
> > > 'panfrost_prime',
> > > diff --git a/tests/nouveau_crc.c b/tests/nouveau_crc.c
> > > new file mode 100644
> > > index 00000000..05c2f4de
> > > --- /dev/null
> > > +++ b/tests/nouveau_crc.c
> > > @@ -0,0 +1,397 @@
> > > +/*
> > > + * Copyright © 2020 Red Hat Inc.
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining
> > > a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the
> > > next
> > > + * paragraph) shall be included in all copies or substantial portions of
> > > the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + */
> > > +
> > > +#include <fcntl.h>
> > > +#include "igt.h"
> > > +#include "igt_sysfs.h"
> > > +
> > > +IGT_TEST_DESCRIPTION(
> > > +"Tests certain aspects of CRC capture that are exclusive to nvidia
> > > hardware, "
> > > +"such as context flipping.");
> > > +
> > > +typedef struct {
> > > + int pipe;
> > > + int drm_fd;
> > > + int nv_crc_dir;
> > > + igt_display_t display;
> > > + igt_output_t *output;
> > > + igt_plane_t *primary;
> > > + drmModeModeInfo *mode;
> > > + igt_fb_t default_fb;
> > > +} data_t;
> > > +
> > > +struct color_fb {
> > > + double r, g, b;
> > > + igt_crc_t crc;
> > > + igt_fb_t fb;
> > > +};
> > > +
> > > +#define HEX_COLOR(r_, g_, b_) \
> > > + { .r = (r_ / 255.0), .g = (g_ / 255.0), .b = (b_ / 255.0) }
> > > +
> > > +static void set_crc_flip_threshold(data_t *data, unsigned int threshold)
> > > +{
> > > + igt_debug("Setting CRC notifier flip threshold to %d\n", threshold);
> > > + igt_assert_lt(0, igt_sysfs_printf(data->nv_crc_dir, "flip_threshold",
> > > "%d", threshold));
> > > +}
> > > +
> > > +static void create_colors(data_t *data,
> > > + struct color_fb *colors,
> > > + size_t len,
> > > + igt_pipe_crc_t *pipe_crc)
> >
> > Not to bikeshed too much, but this function seems to be more about
> > generating CRCs given some pre-existing colors. Maybe
> > create_color_crcs() would be better? And it wouldn't hurt to throw a
> > little docblock on it, even something as short as:
> >
> > /*
> > * Calculate and set the CRC for each color_fb
> > */
> >
> > > +{
> > > + char *crc_str;
> > > +
> > > + igt_pipe_crc_start(pipe_crc);
> > > +
> > > + for (int i = 0; i < len; i++) {
> > > + igt_create_color_fb(data->drm_fd,
> > > + data->mode->hdisplay,
> > > + data->mode->vdisplay,
> > > + DRM_FORMAT_XRGB8888,
> > > + LOCAL_DRM_FORMAT_MOD_NONE,
> > > + colors[i].r, colors[i].g, colors[i].b,
> > > + &colors[i].fb);
> > > +
> > > + igt_plane_set_fb(data->primary, &colors[i].fb);
> > > + igt_display_commit(&data->display);
> > > + igt_pipe_crc_get_current(data->drm_fd, pipe_crc,
> > > &colors[i].crc);
> > > +
> > > + crc_str = igt_crc_to_string(&colors[i].crc);
> > > + igt_debug("CRC for frame %d of pattern: %s\n",
> > > + i, crc_str);
> > > + free(crc_str);
> > > + }
> > > +
> > > + igt_pipe_crc_stop(pipe_crc);
> > > +}
> > > +
> > > +static void destroy_colors(data_t *data, struct color_fb *colors, size_t
> > > len)
> > > +{
> > > + /* So we don't turn off the pipe if we remove it's current fb */
> > > + igt_plane_set_fb(data->primary, &data->default_fb);
> > > +
> > > + for (int i = 0; i < len; i++)
> > > + igt_remove_fb(data->drm_fd, &colors[i].fb);
> > > +}
> > > +
> > > +/* Hard-coded to PIPE_A for now, we don't really need to test this on
> > > more then
> > > + * one head
> > > + */
> >
> > Perhaps expand this doc-block out a bit with the details you have in the
> > commit:
> >
> > /* Test CRCs are reported across notifier context flips
> > *
> > * Nvidia GPUs store CRCs in a limited memory region called the notifier
> > * context. When the region fills, new CRCs are not reported. To work
> > * around this, two notifier contexts are allocated and the driver flips
> > * between the two of them when a threshold is reached. Even with this
> > * approach, a single frame is lost during the context flip.
> > *
> > * This test adjusts the threshold at which the notifier context flips
> > * to the other context and asserts the CRCs are correctly reported with
> > * the exception of the frame lost during the flip.
> > *
> > * Hard-coded to PIPE_A for now, we don't really need to test this on
> > * more then one head
> > */
> >
> > I was guilty of reading the code while testing without going back to
> > consult the commit message and I only just now realized the test purpose
> > was clearly explained. It'd be nice to keep that close to the test
> > definition rather than in the commit log.
> >
> > Also, as far as I can tell it's not actually hard-coded to PIPE_A, but
> > test_ctx_flip_threshold_reset_after_capture() is. Maybe I'm
> > mis-understanding that?
>
> No you aren't-that's definitely a bit of doc I had written before all of the
> tests were complete and I was undecided if we needed to test this on all pipes
> or not (turned out all pipes is a good idea).
>
> >
> > > +static void test_ctx_flip_detection(data_t *data)
> > > +{
> > > + struct color_fb colors[] = {
> > > + HEX_COLOR(0xFF, 0x00, 0x18),
> > > + HEX_COLOR(0xFF, 0xA5, 0x2C),
> > > + HEX_COLOR(0xFF, 0xFF, 0x41),
> > > + HEX_COLOR(0x00, 0x80, 0x18),
> > > + HEX_COLOR(0x00, 0x00, 0xF9),
> > > + HEX_COLOR(0x86, 0x00, 0x7D),
> > > + };
> > > + igt_output_t *output = data->output;
> > > + igt_plane_t *primary = data->primary;
> > > + igt_pipe_crc_t *pipe_crc;
> > > + const int n_colors = ARRAY_SIZE(colors);
> > > + const int n_crcs = 20;
> > > + igt_crc_t *crcs = NULL;
> > > + int start = -1, frame, start_color = -1, i;
> > > + bool found_skip = false;
> > > +
> > > + pipe_crc = igt_pipe_crc_new(data->drm_fd, data->pipe, "auto");
> > > +
> > > + create_colors(data, colors, n_colors, pipe_crc);
> > > +
> > > + set_crc_flip_threshold(data, n_crcs / 2);
> > > + igt_pipe_crc_start(pipe_crc);
> > > +
> > > + for (i = 0; i < n_crcs; i++) {
> > > + const int color_idx = i % n_colors;
> > > +
> > > + igt_plane_set_fb(primary, &colors[color_idx].fb);
> > > + do_or_die(drmModePageFlip(data->drm_fd,
> > > + output->config.crtc->crtc_id,
> > > + colors[color_idx].fb.fb_id,
> > > + DRM_MODE_PAGE_FLIP_EVENT,
> > > + NULL));
> > > + kmstest_wait_for_pageflip(data->drm_fd);
> >
> > While testing I was experiencing some test failures, at which point I
> > turned on a bunch of debug logging on the kernel side. This led to this
> > test reliably failing because this call timed out waiting on the pageflip
> > event, which is unfortunate. I'm not sure what the larger usage of this
> > particular function is, but having tests start to fail when verbose
> > logging is enabled isn't ideal.
> >
> > Obviously hanging forever is also not ideal, so maybe the timeout of
> > this function just needs adjustment, or even just have the assertion
> > include a hint that perhaps debug logging is slowing things down too
> > much. It's not within the scope of this change so I don't think anything
> > needs to change in this patch, more just wanted to see what folks think
> > the best approach is since I spent longer than I care to admit figuring
> > that out and don't want others to have to suffer the same way.
>
> If you turn on enough debugging info it definitely makes sense that it might
> cause us to miss a page flip on accident, although I've never managed to get
> debugging output to slow things down that much. We should probably look to see
> if there's anything we could do to speed up debugging output like that on
> nouveau's side.
>
Well, I did flip on a bunch of debug options so maybe if I turn all
those off it'll pass. Perhaps all IGT tests are sensitive to timing and
have particular requirements when it comes to configuration and so on,
but I think ideally tests should not start to fail in low-performance
environments unless they're specifically about testing performance.
However, I know we don't live in an ideal world.
> JFYI too - we shouldn't ever be missing a pageflip in general and for this
> test, especially under normal circumstances since it sounds like you were
> seeing intermittent failures before you had debugging code turned on? We can't
> accurately figure out if the CRC flip happened otherwise since we rely on
> there being exactly one frame with a missing CRC, with each successive frame
> matching the steps in the color pattern that would have followed that frame.
>
Hmm. What about the frame numbers? I didn't find any documentation on
what to expect for those, but based on what I saw while debugging they
increment and it was clear where the flip happened based on the missing
frame number in the CRCs.
> >
> > > + }
> > > +
> > > + igt_pipe_crc_get_crcs(pipe_crc, n_crcs, &crcs);
> > > + igt_pipe_crc_stop(pipe_crc);
> > > +
> > > + /*
> > > + * Find the first color in our pattern with a CRC that differs from
> > > the
> > > + * last CRC, so we can use it to find the start of the pattern
> > > + */
> >
> > It might be worth explicitly noting this is to avoid CRC collisions (I'm
> > assuming) since I had to ponder for a bit:
> >
> > /*
> > * Guard against CRC collisions in the color framebuffers. Find the
> > * first color in our pattern with a CRC that differs from the last CRC,
> > * so we can use it to find the start of the pattern
> > */
> >
> > > + for (i = 0; i < n_colors - 1; i++) {
> > > + if (igt_check_crc_equal(&colors[i].crc, &colors[n_colors -
> > > 1].crc))
> > > + continue;
> > > +
> > > + igt_debug("Using frame %d of pattern for finding start\n", i);
> > > + start_color = i;
> > > + break;
> > > + }
> > > + igt_assert_lte(0, start_color);
> > > +
> > > + /* Now, figure out where the pattern starts */
> > > + for (i = 0; i < n_crcs; i++) {
> > > + if (!igt_check_crc_equal(&colors[start_color].crc, &crcs[i]))
> > > + continue;
> > > +
> > > + start = i - start_color;
> > > + frame = crcs[i].frame;
> > > + igt_debug("Pattern started on frame %d\n", frame);
> > > + break;
> > > + }
> > > + igt_assert_lte(0, start);
> > > +
> > > + /* And finally, assert that according to the CRCs exactly all but one
> > > + * frame was displayed in order. The missing frame comes from
> > > + * (inevitably) losing a single CRC event when nouveau switches
> > > notifier
> > > + * contexts
> > > + */
> > > + for (i = start; i < n_crcs; i++, frame++) {
> > > + igt_crc_t *crc = &crcs[i];
> > > + char *crc_str;
> > > + int color_idx;
> > > +
> > > + crc_str = igt_crc_to_string(crc);
> > > + igt_debug("CRC %d: vbl=%d val=%s\n", i, crc->frame, crc_str);
> > > + free(crc_str);
> > > +
> > > + if (!found_skip && crc->frame != frame) {
> > > + igt_debug("^^^ Found expected skipped CRC %d ^^^\n",
> > > + crc->frame - 1);
> > > + found_skip = true;
> > > + frame++;
> > > + }
> > > +
> > > + /* We should never skip more then one frame */
> > > + if (found_skip) {
> > > + igt_assert_eq(crc->frame, frame);
> > > + color_idx = (i - start + 1) % n_colors;
> > > + } else {
> > > + color_idx = (i - start) % n_colors;
> > > + }
> > > +
> > > + igt_assert_crc_equal(crc, &colors[color_idx].crc);
> >
> > This consistently fails for me. The reason is that there are multiple
> > frames with the same color/CRC. As far as I can tell, that's perfectly
> > reasonable when a page flip is slow (possibly due to debug logs 😬), but
> > I don't know for certain what the interface expectations are.
>
> Yeah-something is definitely broken with your setup. Page flips should never
> be slow, except -maybe- in the most exceptional of debugging circumstances.
>
> >
> > If that assumption is correct, I guess you'll need to expect either the
> > same color as the previous frame, or the next color in the array after
> > the color in the previous frame.
> >
> > > + }
> > > + igt_assert(found_skip);
> > > +
> > > + free(crcs);
> > > + igt_pipe_crc_free(pipe_crc);
> > > + destroy_colors(data, colors, ARRAY_SIZE(colors));
> > > +}
> > > +
> > > +/* Test whether or not IGT is able to handle frame skips when requesting
> > > the
> > > + * CRC for the current frame
> > > + */
> > > +static void test_ctx_flip_skip_current_frame(data_t *data)
> > > +{
> > > + struct color_fb colors[] = {
> > > + { .r = 1.0, .g = 0.0, .b = 0.0 },
> > > + { .r = 0.0, .g = 1.0, .b = 0.0 },
> > > + { .r = 0.0, .g = 0.0, .b = 1.0 },
> > > + };
> > > + igt_output_t *output = data->output;
> > > + igt_pipe_crc_t *pipe_crc;
> > > + igt_plane_t *primary = data->primary;
> > > + const int fd = data->drm_fd;
> > > + const int n_colors = ARRAY_SIZE(colors);
> > > + const int n_crcs = 30;
> > > +
> > > + pipe_crc = igt_pipe_crc_new(fd, data->pipe, "auto");
> > > + create_colors(data, colors, n_colors, pipe_crc);
> > > +
> > > + set_crc_flip_threshold(data, 5);
> > > + igt_pipe_crc_start(pipe_crc);
> > > +
> > > + for (int i = 0; i < n_crcs; i++) {
> > > + igt_crc_t crc;
> > > + const int color_idx = i % n_colors;
> > > +
> > > + igt_plane_set_fb(primary, &colors[color_idx].fb);
> > > + do_or_die(drmModePageFlip(fd,
> > > + output->config.crtc->crtc_id,
> > > + colors[color_idx].fb.fb_id,
> > > + DRM_MODE_PAGE_FLIP_EVENT,
> > > + NULL));
> > > + kmstest_wait_for_pageflip(fd);
> > > +
> > > + igt_pipe_crc_get_current(fd, pipe_crc, &crc);
> > > + igt_assert_crc_equal(&colors[color_idx].crc, &crc);
> > > + }
> > > +
> > > + igt_pipe_crc_stop(pipe_crc);
> > > + igt_pipe_crc_free(pipe_crc);
> > > + destroy_colors(data, colors, n_colors);
> > > +}
> > > +
> >
> > Maybe a short doc-block with what the test should be doing here would be
> > good:
> >
> > /*
> > * Assert that after a CRC capture is started and stopped, the context
> > * notifier flip threshold is reset to its default value.
> > */
> >
> > > +static void test_ctx_flip_threshold_reset_after_capture(data_t *data)
> > > +{
> > > + igt_pipe_crc_t *pipe_crc;
> > > + const int fd = data->drm_fd;
> > > +
> > > + pipe_crc = igt_pipe_crc_new(fd, data->pipe, "auto");
> > > +
> > > + set_crc_flip_threshold(data, 5);
> > > + igt_pipe_crc_start(pipe_crc);
> > > + igt_pipe_crc_stop(pipe_crc);
> > > +
> > > + igt_assert_neq(igt_sysfs_get_u32(data->nv_crc_dir, "flip_threshold"),
> > > 5);
> >
> > As it currently stands, this test could fail if the default is 5, or it
> > could pass if "flip_threshold" was broken and didn't report the actual
> > flip_threshold anyway. The default being 5 doesn't look to be something
> > that you necessarily need to be worried about, but it might be worth
> > reading the value back after setting the threshold to make sure you do
> > see 5.
> >
> > > + igt_pipe_crc_free(pipe_crc);
> > > +}
> > > +
> >
> > Same note about a docblock 🙂:
> >
> > /*
> > * Assert that, for a given source, two consecutive CRCs are equal if
> > * the content hasn't changed.
> > */
> >
> > > +static void test_source(data_t *data, const char *source)
> > > +{
> > > + igt_pipe_crc_t *pipe_crc = igt_pipe_crc_new(data->drm_fd, data->pipe,
> > > source);
> > > + igt_crc_t *crcs;
> > > +
> > > + igt_pipe_crc_start(pipe_crc);
> > > + igt_pipe_crc_get_crcs(pipe_crc, 2, &crcs);
> > > + igt_pipe_crc_stop(pipe_crc);
> > > +
> > > + /* The CRC shouldn't change if the source content hasn't changed */
> > > + igt_assert_crc_equal(&crcs[0], &crcs[1]);
> > > +
> > > + igt_pipe_crc_free(pipe_crc);
> > > + free(crcs);
> > > +}
> > > +
> >
> > I'm not as certain about what exactly the "outp-inactive" source (or the
> > other CRC sources) is so I'm not sure what to put in the doc-block that
> > I'd recommend putting here.
> >
> > > +static void test_source_outp_inactive(data_t *data)
> > > +{
> > > + struct color_fb colors[] = {
> > > + { .r = 1.0, .g = 0.0, .b = 0.0 },
> > > + { .r = 0.0, .g = 1.0, .b = 0.0 },
> > > + };
> > > + igt_pipe_crc_t *pipe_crc;
> > > + const int fd = data->drm_fd;
> > > + const int n_colors = ARRAY_SIZE(colors);
> > > +
> > > + pipe_crc = igt_pipe_crc_new(fd, data->pipe, "outp-inactive");
> > > + create_colors(data, colors, n_colors, pipe_crc);
> > > +
> > > + /* Changing the color should not change what's outside the active
> > > raster */
> > > + igt_assert_crc_equal(&colors[0].crc, &colors[1].crc);
> > > +
> > > + igt_pipe_crc_free(pipe_crc);
> > > + destroy_colors(data, colors, n_colors);
> > > +}
> > > +
> > > +data_t data = {0, };
> > > +
> > > +#define pipe_test(name) igt_subtest_f("pipe-%s-" name,
> > > kmstest_pipe_name(pipe))
> > > +igt_main
> > > +{
> > > + int pipe;
> > > +
> > > + igt_fixture {
> > > + data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> > > + igt_require_nouveau(data.drm_fd);
> > > +
> > > + kmstest_set_vt_graphics_mode();
> > > +
> > > + igt_require_pipe_crc(data.drm_fd);
> > > + igt_display_require(&data.display, data.drm_fd);
> > > + igt_display_reset(&data.display);
> > > + }
> > > +
> > > + for_each_pipe_static(pipe) {
> > > + igt_fixture {
> > > + int dir;
> > > +
> > > + data.pipe = pipe;
> > > + igt_display_require_output_on_pipe(&data.display,
> > > pipe);
> > > + data.output =
> > > igt_get_single_output_for_pipe(&data.display, pipe);
> > > + data.mode = igt_output_get_mode(data.output);
> > > +
> > > + /* None of these tests need to perform modesets,
> > > + * just page flips. So running display setup
> > > + * here is fine
> > > + */
> > > + igt_output_set_pipe(data.output, pipe);
> > > + data.primary = igt_output_get_plane(data.output, 0);
> > > + igt_create_color_fb(data.drm_fd,
> > > + data.mode->hdisplay,
> > > + data.mode->vdisplay,
> > > + DRM_FORMAT_XRGB8888,
> > > + LOCAL_DRM_FORMAT_MOD_NONE,
> > > + 0.0, 0.0, 0.0,
> > > + &data.default_fb);
> > > + igt_plane_set_fb(data.primary, &data.default_fb);
> > > + igt_display_commit(&data.display);
> > > +
> > > + dir = igt_debugfs_pipe_dir(data.drm_fd, pipe,
> > > O_DIRECTORY);
> > > + igt_require_fd(dir);
> > > + data.nv_crc_dir = openat(dir, "nv_crc", O_DIRECTORY);
> > > + close(dir);
> > > + igt_require_fd(data.nv_crc_dir);
> > > + }
> > > +
> > > + /* We don't need to test this on every pipe, but the
> > > + * setup is the same */
> > > + if (pipe == PIPE_A) {
> > > + igt_describe("Make sure that the CRC notifier context
> > > flip threshold "
> > > + "is reset to its default value after a
> > > single capture.");
> > > + igt_subtest("ctx-flip-threshold-reset-after-capture")
> > > + test_ctx_flip_threshold_reset_after_capture(&d
> > > ata);
> > > + }
> > > +
> > > + igt_describe("Make sure the association between each CRC and
> > > its "
> > > + "respective frame index is not broken when the
> > > driver "
> > > + "performs a notifier context flip.");
> > > + pipe_test("ctx-flip-detection")
> > > + test_ctx_flip_detection(&data);
> > > +
> > > + igt_describe("Make sure that igt_pipe_crc_get_current() works
> > > even "
> > > + "when the CRC for the current frame index is
> > > lost.");
> > > + pipe_test("ctx-flip-skip-current-frame")
> > > + test_ctx_flip_skip_current_frame(&data);
> > > +
> > > + igt_describe("Check that basic CRC readback using the outp-
> > > complete "
> > > + "source works.");
> > > + pipe_test("source-outp-complete")
> > > + test_source(&data, "outp-complete");
> > > +
> > > + igt_describe("Check that basic CRC readback using the rg
> > > source "
> > > + "works.");
> > > + pipe_test("source-rg")
> > > + test_source(&data, "rg");
> > > +
> > > + igt_describe("Make sure that the outp-inactive source is
> > > actually "
> > > + "capturing the inactive raster.");
> > > + pipe_test("source-outp-inactive")
> > > + test_source_outp_inactive(&data);
> > > +
> > > + igt_fixture {
> > > + igt_output_set_pipe(data.output, PIPE_NONE);
> > > + igt_display_commit(&data.display);
> > > + igt_remove_fb(data.drm_fd, &data.default_fb);
> > > + close(data.nv_crc_dir);
> > > + }
> > > + }
> > > + igt_fixture
> > > + igt_display_fini(&data.display);
> > > +
> > > +}
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
_______________________________________________
Nouveau mailing list
Nouveau@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/nouveau
