[igt-dev] [PATCH] lib/igt_debugfs: skip test instead of assert if crc/control is invalid

[Arkadiusz Hiler]

On Fri, Jul 09, 2021 at 11:30:50AM -0400, Mark Yacoub wrote:
> On Fri, Jul 9, 2021 at 8:41 AM Arkadiusz Hiler <arek at hiler.eu> wrote:
> >
> > On Thu, Jul 08, 2021 at 12:01:27PM -0400, Mark Yacoub wrote:
> > > From: Mark Yacoub <markyacoub at google.com>
> > >
> > > [Why]
> > > Some drivers such as msm do not currently support debugfs yet.
> > >
> > > [How]
> > > Change igt_assert of opening "crtc-%d/crc/control" to igt_require_f.
> > > Opening this file doesn't involved a complicated logic so assert isn't
> > > required to verify that things are smooth. -1 indicates that the file
> > > isn't supported by the driver.
> > >
> > > Test: kms_atomic:plane-immutable-zpos on ChromeOS Trogdor(msm)
> >
> > Hey Mark,
> >
> > I am generally not a fan of hidden requirements that are burried deep in
> > the helper libraries. It's nice when the tests explicitly state what they
> > need through the use of functions with _require_ or _skip_ in the name.
> Makes sense to keep visible. thanks!
> >
> > When you probably want to do here is to call igt_require_pipe_crc[1] in
> > plane-immutable-zpos just like the other pipe crc tests do.
> So i was thinking of sending the new patch as a v2 but it's completely
> different change in a different file with a different commit message
> so i thought it makes more sense to make it a completely different
> unrelated patch: https://patchwork.freedesktop.org/series/92369/
> Does this make sense, or a v2 would have been more adequate?

That's makes perfect sense. Thanks for the new patch! :-)

Cheers,
Arek

> >
> > [0]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-debugfs.html#igt-require-pipe-crc
> >
> > --
> > Cheers,
> > Arek