[igt-dev] [PATCH] tests/kms_prime: Filter out devices that can't import buffers.

[Daniel Vetter]

On Wed, Jun 16, 2021 at 11:08 AM Christian König
<christian.koenig at amd.com> wrote:
>
> Am 16.06.21 um 10:46 schrieb Petri Latvala:
> > On Wed, Jun 16, 2021 at 10:07:18AM +0200, Christian König wrote:
> >>
> >> Am 16.06.21 um 09:34 schrieb Petri Latvala:
> >>> On Tue, Jun 15, 2021 at 06:20:23PM +0200, Christian König wrote:
> >>>> The alternative would be to try to create a framebuffer, but don't take it
> >>>> as a failure when that fails with the correct return code.
> >>> This.
> >>>
> >>> If the importing can fail for documented reasons, we should just add
> >>> code that checks that the error is the documented one and then
> >>> igt_skip(), fail the test if it's something it shouldn't.
> >> Good point, but I don't think that this behavior is well documented
> >> anywhere.
> >>
> >> I'm also pretty sure that most drivers don't correctly reject an imported
> >> DMA-buf for scanout when the hardware can't do this. Resulting in either
> >> just garbage on the screen or a lockup.
> >>
> >> Exercising this in an unit tests is probably a good idea, but only after
> >> auditing all drivers for correct behavior.
> > The thing is, having a test for it is the best method of auditing and
> > being _sure_.
>
> Ok, yeah that is a really good argument as well.

Also ideally we pull in a few more compositor folks into the
discussion, and then make sure that the ADDFB kernel documentation is
updated accordingly. Atm it doesn't even really exist, our uapi docs
are a bit bare-bones, but we got to start somewhere.

Also this could already fail at the FD2HANDLE stage in many cases.
E.g. kms-only drivers generally have their "can I scan this out" tests
there, but drivers with kms+render can only check for "can I use this
at all" in FD2HANDLE, and need to delegate "can I scan this out" to
ADDFB.

When we've nailed all that, i.e. igt tests that we agree on and
documented kernel behaviour, then I think this kind of stuff should be
a generic test.

> > Being able to lockup devices with an operation that doesn't even
> > require root (?) sounds... slightly bad.
>
> Well a bit more than slightly bad I would say :)
>
> First we added the extra check to amdgpu because we found screen garbage
> on dGPUs. When we then later implemented scanout from imported DMA-bufs
> for APUs we found that a certain combination of IOMMU enabled,
> resolution and refresh rate could hard lock the PCIe root complex.
>
> It's absolutely not nice if even serial consoles or network consoles
> don't give you any hint on what's going wrong.
>
> I also think we should keep this test case around, just don't be
> surprised if people start to scream because it hard lock their systems.

That sounds like driver bugs to me, but also don't get me started on
hardlocking the box because the scanout block hogs the memory
controller :-)

btw the special drivers are not i915, i915 can pretty much scan out
anything that's in system memory as a igpu (if it's the right format
and alignment ofc). dgpu needs vram. but there's definitely a lot of
igpu that have very special needs indeed and can both fail ad
FD2HANDLE and ADDFB stage for a given buffer simply due to the backing
storage.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch