All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>,
	"Katragadda, MastanX" <mastanx.katragadda@intel.com>
Subject: Re: [igt-dev] [i-g-t] tests/i915/exec_balancer: Added Skip Guc Submission
Date: Wed, 1 Dec 2021 15:57:39 -0800	[thread overview]
Message-ID: <20211201235738.GB36147@jons-linux-dev-box> (raw)
In-Reply-To: <CAKMK7uHN4k9zDEmdQyscj7MsoDFcqVp0P80uEMzHfcaZ8HxnqA@mail.gmail.com>

On Wed, Dec 01, 2021 at 12:46:06PM +0100, Daniel Vetter wrote:
> On Wed, Dec 1, 2021 at 12:37 PM Radoslaw Szwichtenberg
> <radoslaw.szwichtenberg@intel.com> wrote:
> > On 01/12/2021 10:46, Tvrtko Ursulin wrote:
> > > On 30/11/2021 16:48, Matthew Brost wrote:
> > >>
> > >> IMO this fix is 100% correct as this is a known, tracked issue. It was
> > >> agreed upon (arch, i915, GuC team) that we just skip these tests with
> > >> GuC submission.
> > This does not look like a fix to me - you just disable test to hide the
> > result. If this issue is recorded with a bug, is tracked - why cant we
> > just let this test fail till we get this issue fixed?
> 
> This is correct in general, but sadly not for gem igts and selftests.
> The state of our validation suite is screwed up enough that
> unfortunately the safe starting point for failing tests is that the
> test is simply wrong, or too much just validating implementation
> details of the current platform/driver, while not actually validating
> stuff that should be tested for.
> 

That is exactly what this test is doing - IMO it is validating details
of backend scheduler not ABI nor a POR arch requirement. 

> > > I915 team is here on upstream as well.
> > >
> > > Record those acks publicly would be my ask. Unless some security by
> > > obscurity is happening here? Until then from me it is a soft nack to
> > > keep disabling tests which show genuine weaknesses in GuC mode. Soft
> > > until we get a public record of exactly what is broken and in what
> > > circumstances, acked by architects publicly as you say they acked it
> > > somewhere. Commit message devoid of detail is not good enough.
> > This should be most probably documented in the bug, right? Here we
> > should just keep the test as is till the issue is fixed. I don't see how
> > docummenting an issue would enable us to just disable the test.
> 
> Sadly the situation is bad enough that I'm tempted to just drop a few
> thousand Acked-by: me tags in this thread for any case where a
> questionable testcase gets in the way. Unless someone can proof that
> it's a POR architectural requirement we're validating here.
>

Agree our selftests / IGTs are a complete mess with many questionable
test cases. You wouldn't believe the amount of time / effort we waste
looking into these.

> I do agree though that really we should just delete such tests
> outright, not hide the mess on each platform individually.

Agree likely should do an audit of our tests and delete some of them.

Matt

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

  parent reply	other threads:[~2021-12-02  0:05 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-26  1:57 [igt-dev] [i-g-t] tests/i915/exec_balancer: Added Skip Guc Submission Mastan Katragadda
2021-11-26  2:42 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2021-11-26  8:46 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2021-11-29 10:30 ` [igt-dev] [i-g-t] " Tvrtko Ursulin
2021-11-29 10:58   ` Katragadda, MastanX
2021-11-29 11:15     ` Tvrtko Ursulin
2021-11-30 16:48       ` Matthew Brost
2021-12-01  9:46         ` Tvrtko Ursulin
2021-12-01 11:36           ` Radoslaw Szwichtenberg
2021-12-01 11:46             ` Daniel Vetter
2021-12-01 12:14               ` Tvrtko Ursulin
2021-12-01 23:57               ` Matthew Brost [this message]
2021-12-01 23:42           ` Matthew Brost
2021-12-02  9:19             ` Petri Latvala
2021-12-02  9:20             ` Tvrtko Ursulin
2021-12-03 19:36             ` Matthew Brost
2021-12-09  8:20 Mastan Katragadda
2021-12-10  3:20 ` Matthew Brost
2021-12-10 10:02 ` Tvrtko Ursulin
2021-12-22  9:30   ` Tvrtko Ursulin
2021-12-22 21:02     ` John Harrison
2021-12-23 11:08       ` Tvrtko Ursulin
2021-12-23 11:16         ` Surendrakumar Upadhyay, TejaskumarX

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=20211201235738.GB36147@jons-linux-dev-box \
    --to=matthew.brost@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=mastanx.katragadda@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.