[Intel-gfx] [PATCH 1/2] drm/i915: Add MOCS state dump to debugfs

[Chris Wilson]

Quoting Stuart Summers (2019-08-08 00:00:17)
> On Wed, 2019-08-07 at 23:01 +0100, Chris Wilson wrote:
> > Quoting Stuart Summers (2019-08-07 22:48:55)
> > > On Wed, 2019-08-07 at 22:29 +0100, Chris Wilson wrote:
> > > > Quoting Stuart Summers (2019-08-07 21:55:55)
> > > > > User applications might need to verify hardware configuration
> > > > > of the MOCS entries. To facilitate this debug, add a new
> > > > > debugfs
> > > > > entry to allow a dump of the MOCS state to verify expected
> > > > > values
> > > > > are set by i915.
> > > > 
> > > > User applications + debugfs? It's not an avenue for ABI.
> > > > 
> > > > If you really want to provide the settings back to userspace,
> > > > look at
> > > > something like an i915_query or sysfs.
> > > > 
> > > > Or if you just mean igt, then add a Testcase:
> > > > 
> > > > If you just need to validate that we are setting and restoring
> > > > them,
> > > > selftests.
> > > > 
> > > > If you need them for debugging errors, add them to the error
> > > > state.
> > > 
> > > This was probably poorly worded, you're right. I'll update the
> > > commit
> > > message to be more specific.
> > > 
> > > I do want this for debugging, but not sure error state is the right
> > > place. This is for debugging performance issues, so no specific
> > > failures. If you feel sysfs or i915_query are more correct here, I
> > > can
> > > look at adding this there instead. Is there a reason we don't want
> > > this
> > > in debugfs specifically?
> > 
> > No, it was just the wording implied to me you had a use case for
> > clients, not just debugging the kernel.
> > 
> > Adding it to the error state (see i915_gpu_info) is not too bad an
> > idea
> > if you need a sledgehammer to inspect the GPU state while a batch is
> > executing, but really it just sounds like you want to automate
> > checking
> > the mocs registers against "ideal" state. They should be static, so
> > once
> > they are set, so long as we are confident and check that they do not
> > change nor can be scribbled over by userspace, you only need to scan
> > the
> > source :)
> > 
> > I will add that I wish we took a more complete snapshot of
> > interesting
> > registers for the error state.
> 
> I guess my question is about intent of the error state. I can add it
> there, but do we want this to indicate any register state we might want
> to investigate, even if the registers are "correct", but just need
> review based on current behavior?

It was created for debugging userspace batches (later added to hang
detection as a means of automatically grabbing the hopefully relevant
batch). As such it's a motley collection of information that at some
point proved useful. If you can make use of it, and find it more useful
to have the mocs registers in the same snapshot as the user batch,
please do include it. (Fwiw, I would like to extend the error state with
a bunch of { offset:0xfoo, value:0xbar } given a set of tables listing
the interesting regs. There just hasn't been an urgent need. Also on
that wishlist is devcoredump.)
-Chris