[Intel-gfx] [PATCH] drm/i915: read/write IOCTLs

[Eric Anholt]

On Fri, 01 Apr 2011 08:32:09 +0100, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Fri, 1 Apr 2011 00:06:37 -0700, Ben Widawsky <ben at bwidawsk.net> wrote:
> > On Fri, Apr 01, 2011 at 07:36:56AM +0100, Chris Wilson wrote:
> > > Nice. The consensus is that this ioctl is required, just a few comments
> > > inline.
> > 
> > Nobody likes it, but it could pay off, especially if we end up with
> > weird non-mappable registers we wish to allow user space to read. I had
> > actually decided to add another field (something like a bus) to the
> > interface just in case.
> 
> I think most of that can and should be done in userspace. Eventually we
> should have the list of registers and be able to use the spec names and
> have those translated into the correct offset for the chipset.
> 
> Has anyone succeeded in getting a machine-readable description of the
> registers? Or am I just dreaming that we have real documentation
> somewhere?
> 
> > This was a bug in my patch... you mean?
> > ret = mutex_lock_interruptible();
> > if (ret)
> > 	reg_read
> > 
> > if (!ret)
> > 	mutex_unlock()
> 
> Slightly better in avoiding the kernel killer. ;-)
> 
> I'm just not happy about haphazard locking. Can we do simple and safe
> locking and revisit it if a real use-case for brute-forcing the read/write
> is found?

The concern we had was "I want to be sure that when the GPU is hung we
can still reg_dump so people can write bug reports.  The mutex lock try
should have a timeout, and we should go ahead and just try forcewaking
and reading the reg anyway after a while."

> > > > +struct drm_intel_read_reg {
> > > > +	/* register offset to read */
> > > > +	__u32 offset;
> > > > +
> > > > +	/* register size, RFU */
> > > > +	__u8 size;
> > > > +
> > > > +	/* return value, high 4 bytes RFU */
> > > > +	__u64 value;
> > > > +};
> > > 
> > > Haha! I was going to ask for this to handle arbitrary register sizes.
> > > What I think is also necessary is to be able to read/write a block of
> > > registers in a single call - thinking of some of the more complex
> > > procedures we may want to try from userspace (the pcode messages for
> > > example) or for snapshotting a set of registers.
> > 
> > I brought this question up at our meeting on Thursday. The consensus to
> > was just have the 1 register at a time because the overhead of the
> > syscall was small enough to make it not matter. I think I prefer the
> > single read/write a little bit just because it keeps the kernel code
> > simpler, but I don't care enough to argue, and I most certainly don't
> > care enough to set up some kind of performance test for it. Anyone else
> > care enough to argue?
> 
> It's not the performance I care about, but the atomicity. There seem to be
> a growing abundance of messaging systems within the chip driven by
> combinations of registers. I'd feel happier if we could send a message
> without fouling or being fouled by the kernel.

Generally for those things, you end up needing to poll in the middle.
Let's not build a more complicated interface than required for current
use-cases in userland (reading many registers, or writing an arbitrary
one), without solving a specific problem.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20110401/530cb54b/attachment.sig>