On Mon, Sep 04, 2017 at 03:09:10PM -0300, Paulo Zanoni wrote:
> Em Seg, 2017-09-04 às 10:00 +0200, Daniel Vetter escreveu:
> > On Fri, Sep 01, 2017 at 04:44:38PM -0300, Paulo Zanoni wrote:
> > > Em Seg, 2017-08-14 às 11:25 +0200, Daniel Vetter escreveu:
> > > > Macros that should be C functions but aren't are really hard to
> > > > read and confusing. Convert them over.
> > > >
> > > > v2: Clean up commit message and keep printing the line numbers
> > > > (Paulo).
> > > >
> > > > v3: Actually git add (silly me).
> > > >
> > > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > >
> > > Thanks for doing that. Works for me this way:
> > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> >
> > Thanks, pushed.
> >
> > > But I'm still waiting for the patch that removes those bogus line
> > > numbers in every error message we print :).
> >
> > Hm, which bogus line numbers where?
>
> Every igt_log message is starts with:
> (kms_frontbuffer_tracking:XXXX) where XXXX is a number that means
> nothing but looks like a line number, except that that line usually has
> nothing to do with the message.
It's the process id. Not idea how you came up with the idea it's a line
number, executable:pid is pretty standard format for logfiles. Note how
it's the executable file name, not the source code file name.
And pid matters if you have a testcase that does lots of forking.
Should we improve the documentation about this? Care to type a patch
please?
Thanks, Daniel
>
> > -Daniel
> > >
> > > > ---
> > > > tests/kms_frontbuffer_tracking.c | 171 ++++++++++++++++++++-----
> > > > ----
> > > > ----------
> > > > 1 file changed, 89 insertions(+), 82 deletions(-)
> > > >
> > > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > > b/tests/kms_frontbuffer_tracking.c
> > > > index e03524f1c45b..a068c8afb6d8 100644
> > > > --- a/tests/kms_frontbuffer_tracking.c
> > > > +++ b/tests/kms_frontbuffer_tracking.c
> > > > @@ -1677,88 +1677,95 @@ static int adjust_assertion_flags(const
> > > > struct test_mode *t, int flags)
> > > > return flags;
> > > > }
> > > >
> > > > -#define do_crc_assertions(flags, mandatory_sink_crc) do {
> > > >
> > > > \
> > > > - int flags__ = (flags);
> > > > \
> > > > - struct both_crcs crc_;
> > > > \
> > > > -
> > > > \
> > > > - if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))
> > > >
> > > > \
> > > > - break;
> > > > \
> > > > -
> > > > \
> > > > - collect_crcs(&crc_, mandatory_sink_crc);
> > > > \
> > > > - print_crc("Calculated CRC:", &crc_);
> > > >
> > > > \
> > > > -
> > > > \
> > > > - igt_assert(wanted_crc);
> > > > \
> > > > - igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe);
> > > >
> > > > \
> > > > - if (mandatory_sink_crc)
> > > > \
> > > > - assert_sink_crc_equal(&crc_.sink, &wanted_crc-
> > > > > sink); \
> > > >
> > > > - else if (sink_crc.reliable &&
> > > > \
> > > > - !sink_crc_equal(&crc_.sink, &wanted_crc->sink))
> > > >
> > > > \
> > > > - igt_info("Sink CRC differ, but not
> > > > required\n");
> > > > \
> > > > -} while (0)
> > > > -
> > > > -#define do_status_assertions(flags_) do {
> > > >
> > > > \
> > > > - if (!opt.check_status) {
> > > > \
> > > > - /* Make sure we settle before continuing. */
> > > >
> > > > \
> > > > - sleep(1);
> > > >
> > > > \
> > > > - break;
> > > > \
> > > > - }
> > > >
> > > > \
> > > > -
> > > > \
> > > > - if (flags_ & ASSERT_FBC_ENABLED) {
> > > >
> > > > \
> > > > - igt_require(!fbc_not_enough_stolen());
> > > > \
> > > > - igt_require(!fbc_stride_not_supported());
> > > >
> > > > \
> > > > - if (!fbc_wait_until_enabled()) {
> > > > \
> > > > - fbc_print_status();
> > > >
> > > > \
> > > > - igt_assert_f(false, "FBC disabled\n");
> > > > \
> > > > - }
> > > >
> > > > \
> > > > -
> > > > \
> > > > - if (opt.fbc_check_compression)
> > > > \
> > > > - igt_assert(fbc_wait_for_compression());
> > > > \
> > > > - } else if (flags_ & ASSERT_FBC_DISABLED) {
> > > >
> > > > \
> > > > - igt_assert(!fbc_wait_until_enabled());
> > > > \
> > > > - }
> > > >
> > > > \
> > > > -
> > > > \
> > > > - if (flags_ & ASSERT_PSR_ENABLED) {
> > > >
> > > > \
> > > > - if (!psr_wait_until_enabled()) {
> > > > \
> > > > - psr_print_status();
> > > >
> > > > \
> > > > - igt_assert_f(false, "PSR disabled\n");
> > > > \
> > > > - }
> > > >
> > > > \
> > > > - } else if (flags_ & ASSERT_PSR_DISABLED) {
> > > >
> > > > \
> > > > - igt_assert(!psr_wait_until_enabled());
> > > > \
> > > > - }
> > > >
> > > > \
> > > > -} while (0)
> > > > -
> > > > -#define do_assertions(flags) do {
> > > >
> > > > \
> > > > - int flags_ = adjust_assertion_flags(t, (flags));
> > > > \
> > > > - bool mandatory_sink_crc = t->feature & FEATURE_PSR;
> > > >
> > > > \
> > > > -
> > > > \
> > > > - wait_user(2, "Paused before assertions.");
> > > >
> > > > \
> > > > -
> > > > \
> > > > - /* Check the CRC to make sure the drawing operations
> > > > work
> > > > \
> > > > - * immediately, independently of the features being
> > > > enabled.
> > > > */ \
> > > > - do_crc_assertions(flags_, mandatory_sink_crc);
> > > > \
> > > > -
> > > > \
> > > > - /* Now we can flush things to make the test faster. */
> > > > \
> > > > - do_flush(t);
> > > >
> > > > \
> > > > -
> > > > \
> > > > - do_status_assertions(flags_);
> > > > \
> > > > -
> > > > \
> > > > - /* Check CRC again to make sure the compressed screen is
> > > > ok,
> > > > \
> > > > - * except if we're not drawing on the primary screen. On
> > > > this \
> > > > - * case, the first check should be enough and a new CRC
> > > > check \
> > > > - * would only delay the test suite while adding no value
> > > > to
> > > > the \
> > > > - * test suite. */
> > > >
> > > > \
> > > > - if (t->screen == SCREEN_PRIM)
> > > > \
> > > > - do_crc_assertions(flags_, mandatory_sink_crc);
> > > > \
> > > > -
> > > > \
> > > > - if (fbc.supports_last_action &&
> > > > opt.fbc_check_last_action) {
> > > > \
> > > > - if (flags_ & ASSERT_LAST_ACTION_CHANGED)
> > > > \
> > > > - igt_assert(fbc_last_action_changed());
> > > > \
> > > > - else if (flags_ & ASSERT_NO_ACTION_CHANGE)
> > > >
> > > > \
> > > > - igt_assert(!fbc_last_action_changed());
> > > > \
> > > > - }
> > > >
> > > > \
> > > > -
> > > > \
> > > > - wait_user(1, "Paused after assertions.");
> > > >
> > > > \
> > > > -} while (0)
> > > > +static void do_crc_assertions(int flags, bool
> > > > mandatory_sink_crc)
> > > > +{
> > > > + struct both_crcs crc;
> > > > +
> > > > + if (!opt.check_crc || (flags & DONT_ASSERT_CRC))
> > > > + return;
> > > > +
> > > > + collect_crcs(&crc, mandatory_sink_crc);
> > > > + print_crc("Calculated CRC:", &crc);
> > > > +
> > > > + igt_assert(wanted_crc);
> > > > + igt_assert_crc_equal(&crc.pipe, &wanted_crc->pipe);
> > > > + if (mandatory_sink_crc)
> > > > + assert_sink_crc_equal(&crc.sink, &wanted_crc-
> > > > >sink);
> > > > + else if (sink_crc.reliable &&
> > > > + !sink_crc_equal(&crc.sink, &wanted_crc->sink))
> > > > + igt_info("Sink CRC differ, but not required\n");
> > > > +}
> > > > +
> > > > +static void do_status_assertions(int flags)
> > > > +{
> > > > + if (!opt.check_status) {
> > > > + /* Make sure we settle before continuing. */
> > > > + sleep(1);
> > > > + return;
> > > > + }
> > > > +
> > > > + if (flags & ASSERT_FBC_ENABLED) {
> > > > + igt_require(!fbc_not_enough_stolen());
> > > > + igt_require(!fbc_stride_not_supported());
> > > > + if (!fbc_wait_until_enabled()) {
> > > > + fbc_print_status();
> > > > + igt_assert_f(false, "FBC disabled\n");
> > > > + }
> > > > +
> > > > + if (opt.fbc_check_compression)
> > > > + igt_assert(fbc_wait_for_compression());
> > > > + } else if (flags & ASSERT_FBC_DISABLED) {
> > > > + igt_assert(!fbc_wait_until_enabled());
> > > > + }
> > > > +
> > > > + if (flags & ASSERT_PSR_ENABLED) {
> > > > + if (!psr_wait_until_enabled()) {
> > > > + psr_print_status();
> > > > + igt_assert_f(false, "PSR disabled\n");
> > > > + }
> > > > + } else if (flags & ASSERT_PSR_DISABLED) {
> > > > + igt_assert(!psr_wait_until_enabled());
> > > > + }
> > > > +}
> > > > +
> > > > +static void __do_assertions(const struct test_mode *t, int
> > > > flags,
> > > > + int line)
> > > > +{
> > > > + flags = adjust_assertion_flags(t, flags);
> > > > + bool mandatory_sink_crc = t->feature & FEATURE_PSR;
> > > > +
> > > > + igt_debug("checking asserts in line %i\n", line);
> > > > +
> > > > + wait_user(2, "Paused before assertions.");
> > > > +
> > > > + /* Check the CRC to make sure the drawing operations
> > > > work
> > > > + * immediately, independently of the features being
> > > > enabled.
> > > > */
> > > > + do_crc_assertions(flags, mandatory_sink_crc);
> > > > +
> > > > + /* Now we can flush things to make the test faster. */
> > > > + do_flush(t);
> > > > +
> > > > + do_status_assertions(flags);
> > > > +
> > > > + /* Check CRC again to make sure the compressed screen is
> > > > ok,
> > > > + * except if we're not drawing on the primary screen. On
> > > > this
> > > > + * case, the first check should be enough and a new CRC
> > > > check
> > > > + * would only delay the test suite while adding no value
> > > > to
> > > > the
> > > > + * test suite. */
> > > > + if (t->screen == SCREEN_PRIM)
> > > > + do_crc_assertions(flags, mandatory_sink_crc);
> > > > +
> > > > + if (fbc.supports_last_action &&
> > > > opt.fbc_check_last_action) {
> > > > + if (flags & ASSERT_LAST_ACTION_CHANGED)
> > > > + igt_assert(fbc_last_action_changed());
> > > > + else if (flags & ASSERT_NO_ACTION_CHANGE)
> > > > + igt_assert(!fbc_last_action_changed());
> > > > + }
> > > > +
> > > > + wait_user(1, "Paused after assertions.");
> > > > +}
> > > > +
> > > > +#define do_assertions(__flags) __do_assertions(t, (__flags),
> > > > __LINE__)
> > > >
> > > > static void enable_prim_screen_and_wait(const struct test_mode
> > > > *t)
> > > > {
> >
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
