Re: [PATCH] drm/i915: add register read IOCTL

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 1355 bytes --]

Ben Widawsky <ben@bwidawsk.net> writes:

> The interface's immediate purpose is to do synchronous timestamp queries
> as required by GL_TIMESTAMP. The GPU has a register for reading the
> timestamp but because that would normally require root access, the
> IOCTL can provide this service.
>
> Currently the implementation whitelists only the render ring timestamp
> register, because that is the only thing we need to expose at this time.

Thanks. I was just writing this patch yesterday since it still hadn't
landed.  What I was doing was very similar, I was just not including a
size, since we're going to whitelist regs and the correct size is
implied by the register offset.

> +int i915_reg_read_ioctl(struct drm_device *dev,
> +			void *data, struct drm_file *file)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_reg_read *reg = data;
> +
> +	/* Whitelisted for now */
> +	if (reg->offset != RING_TIMESTAMP(RENDER_RING_BASE))
> +		return -ENXIO;

Should this be conditional on the gen having the timestamp register?

> +struct drm_i915_reg_read {
> +	__u64 offset;
> +	__u32 size;
> +	__u64 val; /* Return value */
> +	__u32 pad;
> +};

Bad padding here.  On i386 you'll get a struct like:

{
        uint64_t offset
        uint32_t size
        uint32_t implicit_pad
        uint64_t val
        uint32_t pad
}

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx