[igt-dev] [PATCH i-g-t 00/12] DG1/LMEM uAPI basics

[Petri Latvala]

On Wed, May 19, 2021 at 11:45:17AM +0100, Matthew Auld wrote:
> On Wed, 19 May 2021 at 09:49, Petri Latvala <petri.latvala at intel.com> wrote:
> >
> > On Wed, May 19, 2021 at 09:13:37AM +0100, Matthew Auld wrote:
> > > On Tue, 11 May 2021 at 17:52, Matthew Auld <matthew.auld at intel.com> wrote:
> > > >
> > > > Just the really basic stuff, which unlocks adding more interesting testcases
> > > > later, like gem_lmem_swapping.
> > > >
> > > > On the kernel side we landed the uAPI bits[1] behind CONFIG_BROKEN, which is
> > > > already enabled in CI builds, so it should be possible to get some more BAT
> > > > testing(outside of just the selftests) on DG1 to the point where we can start to
> > > > exercise the LMEM paths with the new bits of uAPI.
> > > >
> > > > [1] https://patchwork.freedesktop.org/series/89648/
> > >
> > > Petri, any thoughts on this series? As an initial step we just need
> > > some way to start exercising the new bits of uAPI, and from that we
> > > can add more interesting test cases.
> >
> > This series is in a confused state. First there's the addition of
> > local definitions and ioctl tokens, then they are replaced with the
> > proper stuff...
> 
> Yeah, I think that's how it is internally. Maybe the idea with that
> was to somehow land the igt changes first, before the kernel stuff
> potentially landed? I can clean this up and just start with the proper
> stuff.
> 
> >
> > When this was starting to get developed the idea was to avoid icky
> > cases that break _testing_. Not tests, testing. Remember when engine
> > discovery was being developed and we had cases where for_each_engine
> > loop didn't progress, causing stuff like
> >
> > for_each_engine(...)
> >  igt_subtest(...)
> >
> > to never enter a subtest?
> >
> > Pushing for stubbing memory regions ultimately wanted to avoid cases
> > where for_each_combination(memregions) breaks test enumeration.
> >
> > It all boils down to: Can that break? Can we have cases where the
> > query gives garbage? Can it give two million smem regions? Can it give
> > 0 regions, or -1 regions? And what happens then?
> 
> On integrated platforms it can only return one region: smem. If we
> somehow don't have a smem region then the i915 module load would have
> failed, since we must have been unable to populate the
> i915->mm.regions. On DG1 we just get the extra lmem region, and for Xe
> HP multi-tile we get a few more lmem regions, but again if we can't
> populate the i915->mm.regions with the required regions then we fail
> driver init. The only "optional" region is stolen memory, but for that
> we don't expose it to userspace.
> 
> The query will fail on !CONFIG_BROKEN kernels though, where it just
> returns -ENODEV, or of course some other error if the user provided an
> invalid query.

Behaviour between success/failure is business as usual. The danger in
the initial discussions for this was token value overloading or such,
stuff like IGT thinking it's calling DRM_IOCTL_DISTANCE_TO_LUNCHTIME
but that value was meanwhile taken by
DRM_IOCTL_HALT_AND_CATCH_FIRE. Of course the query change is not a new
ioctl but is value mismatch a possibility in a theoretical worst case
and how does the breakage show in testing?


-- 
Petri Latvala