[PATCH 1/3] Drop "-Wunsafe-loop-optimizations".

[PATCH 1/3] Drop "-Wunsafe-loop-optimizations".

[Eric Anholt]

It warns about totally sensible things done in intel_decode.c.  I've
never seen this warn do anything useful, and apparently I was the one
to introduce it when I added the giant pile of warning flags back in
2008.
---
 configure.ac |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 09fed53..3eaec74 100644
--- a/configure.ac
+++ b/configure.ac
@@ -133,7 +133,7 @@ MAYBE_WARN="-Wall -Wextra \
 -Wpointer-arith -Wwrite-strings -Wstrict-prototypes \
 -Wmissing-prototypes -Wmissing-declarations -Wnested-externs \
 -Wpacked -Wswitch-enum -Wmissing-format-attribute \
--Wstrict-aliasing=2 -Winit-self -Wunsafe-loop-optimizations \
+-Wstrict-aliasing=2 -Winit-self \
 -Wdeclaration-after-statement -Wold-style-definition \
 -Wno-missing-field-initializers -Wno-unused-parameter \
 -Wno-attributes -Wno-long-long -Winline"
-- 
1.7.10.4

[PATCH 2/3] intel: Import updated i915_drm.h.

[Eric Anholt]

---
 include/drm/i915_drm.h |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 5c8fabe..7e9e9bd 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -195,6 +195,9 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_GEM_WAIT	0x2c
 #define DRM_I915_GEM_CONTEXT_CREATE	0x2d
 #define DRM_I915_GEM_CONTEXT_DESTROY	0x2e
+#define DRM_I915_GEM_SET_CACHEING	0x2f
+#define DRM_I915_GEM_GET_CACHEING	0x30
+#define DRM_I915_REG_READ		0x31
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -219,6 +222,8 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_PIN		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_PIN, struct drm_i915_gem_pin)
 #define DRM_IOCTL_I915_GEM_UNPIN	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_UNPIN, struct drm_i915_gem_unpin)
 #define DRM_IOCTL_I915_GEM_BUSY		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_BUSY, struct drm_i915_gem_busy)
+#define DRM_IOCTL_I915_GEM_SET_CACHEING		DRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_SET_CACHEING, struct drm_i915_gem_cacheing)
+#define DRM_IOCTL_I915_GEM_GET_CACHEING		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_GET_CACHEING, struct drm_i915_gem_cacheing)
 #define DRM_IOCTL_I915_GEM_THROTTLE	DRM_IO ( DRM_COMMAND_BASE + DRM_I915_GEM_THROTTLE)
 #define DRM_IOCTL_I915_GEM_ENTERVT	DRM_IO(DRM_COMMAND_BASE + DRM_I915_GEM_ENTERVT)
 #define DRM_IOCTL_I915_GEM_LEAVEVT	DRM_IO(DRM_COMMAND_BASE + DRM_I915_GEM_LEAVEVT)
@@ -241,6 +246,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_WAIT		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_WAIT, struct drm_i915_gem_wait)
 #define DRM_IOCTL_I915_GEM_CONTEXT_CREATE	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create)
 #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy)
+#define DRM_IOCTL_I915_REG_READ			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read)
 
 /* Allow drivers to submit batchbuffers directly to hardware, relying
  * on the security mechanisms provided by hardware.
@@ -690,10 +696,31 @@ struct drm_i915_gem_busy {
 	/** Handle of the buffer to check for busy */
 	__u32 handle;
 
-	/** Return busy status (1 if busy, 0 if idle) */
+	/** Return busy status (1 if busy, 0 if idle).
+	 * The high word is used to indicate on which rings the object
+	 * currently resides:
+	 *  16:31 - busy (r or r/w) rings (16 render, 17 bsd, 18 blt, etc)
+	 */
 	__u32 busy;
 };
 
+#define I915_CACHEING_NONE		0
+#define I915_CACHEING_CACHED		1
+
+struct drm_i915_gem_cacheing {
+	/**
+	 * Handle of the buffer to set/get the cacheing level of. */
+	__u32 handle;
+
+	/**
+	 * Cacheing level to apply or return value
+	 *
+	 * bits0-15 are for generic cacheing control (i.e. the above defined
+	 * values). bits16-31 are reserved for platform-specific variations
+	 * (e.g. l3$ caching on gen7). */
+	__u32 cacheing;
+};
+
 #define I915_TILING_NONE	0
 #define I915_TILING_X		1
 #define I915_TILING_Y		2
@@ -910,4 +937,8 @@ struct drm_i915_gem_context_destroy {
 	__u32 pad;
 };
 
+struct drm_i915_reg_read {
+	__u64 offset;
+	__u64 val; /* Return value */
+};
 #endif				/* _I915_DRM_H_ */
-- 
1.7.10.4

Re: [PATCH 2/3] intel: Import updated i915_drm.h.

[Ben Widawsky]

On 2012-08-02 11:29, Eric Anholt wrote:

Yikes, who spelled caching wrong?
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  include/drm/i915_drm.h |   33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 5c8fabe..7e9e9bd 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -195,6 +195,9 @@ typedef struct _drm_i915_sarea {
>  #define DRM_I915_GEM_WAIT	0x2c
>  #define DRM_I915_GEM_CONTEXT_CREATE	0x2d
>  #define DRM_I915_GEM_CONTEXT_DESTROY	0x2e
> +#define DRM_I915_GEM_SET_CACHEING	0x2f
> +#define DRM_I915_GEM_GET_CACHEING	0x30
> +#define DRM_I915_REG_READ		0x31
>
>  #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE +
> DRM_I915_INIT, drm_i915_init_t)
>  #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + 
> DRM_I915_FLUSH)
> @@ -219,6 +222,8 @@ typedef struct _drm_i915_sarea {
>  #define DRM_IOCTL_I915_GEM_PIN		DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_PIN, struct drm_i915_gem_pin)
>  #define DRM_IOCTL_I915_GEM_UNPIN	DRM_IOW(DRM_COMMAND_BASE +
> DRM_I915_GEM_UNPIN, struct drm_i915_gem_unpin)
>  #define DRM_IOCTL_I915_GEM_BUSY		DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_BUSY, struct drm_i915_gem_busy)
> +#define DRM_IOCTL_I915_GEM_SET_CACHEING		DRM_IOW(DRM_COMMAND_BASE +
> DRM_I915_GEM_SET_CACHEING, struct drm_i915_gem_cacheing)
> +#define DRM_IOCTL_I915_GEM_GET_CACHEING		DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_GET_CACHEING, struct drm_i915_gem_cacheing)
>  #define DRM_IOCTL_I915_GEM_THROTTLE	DRM_IO ( DRM_COMMAND_BASE +
> DRM_I915_GEM_THROTTLE)
>  #define DRM_IOCTL_I915_GEM_ENTERVT	DRM_IO(DRM_COMMAND_BASE +
> DRM_I915_GEM_ENTERVT)
>  #define DRM_IOCTL_I915_GEM_LEAVEVT	DRM_IO(DRM_COMMAND_BASE +
> DRM_I915_GEM_LEAVEVT)
> @@ -241,6 +246,7 @@ typedef struct _drm_i915_sarea {
>  #define DRM_IOCTL_I915_GEM_WAIT		DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_WAIT, struct drm_i915_gem_wait)
>  #define DRM_IOCTL_I915_GEM_CONTEXT_CREATE	DRM_IOWR (DRM_COMMAND_BASE
> + DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create)
>  #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY	DRM_IOW (DRM_COMMAND_BASE
> + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy)
> +#define DRM_IOCTL_I915_REG_READ			DRM_IOWR (DRM_COMMAND_BASE +
> DRM_I915_REG_READ, struct drm_i915_reg_read)
>
>  /* Allow drivers to submit batchbuffers directly to hardware, 
> relying
>   * on the security mechanisms provided by hardware.
> @@ -690,10 +696,31 @@ struct drm_i915_gem_busy {
>  	/** Handle of the buffer to check for busy */
>  	__u32 handle;
>
> -	/** Return busy status (1 if busy, 0 if idle) */
> +	/** Return busy status (1 if busy, 0 if idle).
> +	 * The high word is used to indicate on which rings the object
> +	 * currently resides:
> +	 *  16:31 - busy (r or r/w) rings (16 render, 17 bsd, 18 blt, etc)
> +	 */
>  	__u32 busy;
>  };
>
> +#define I915_CACHEING_NONE		0
> +#define I915_CACHEING_CACHED		1
> +
> +struct drm_i915_gem_cacheing {
> +	/**
> +	 * Handle of the buffer to set/get the cacheing level of. */
> +	__u32 handle;
> +
> +	/**
> +	 * Cacheing level to apply or return value
> +	 *
> +	 * bits0-15 are for generic cacheing control (i.e. the above 
> defined
> +	 * values). bits16-31 are reserved for platform-specific variations
> +	 * (e.g. l3$ caching on gen7). */
> +	__u32 cacheing;
> +};
> +
>  #define I915_TILING_NONE	0
>  #define I915_TILING_X		1
>  #define I915_TILING_Y		2
> @@ -910,4 +937,8 @@ struct drm_i915_gem_context_destroy {
>  	__u32 pad;
>  };
>
> +struct drm_i915_reg_read {
> +	__u64 offset;
> +	__u64 val; /* Return value */
> +};
>  #endif				/* _I915_DRM_H_ */

Re: [PATCH 2/3] intel: Import updated i915_drm.h.

[Eric Anholt]

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

Ben Widawsky <ben@bwidawsk.net> writes:

> On 2012-08-02 11:29, Eric Anholt wrote:
>
> Yikes, who spelled caching wrong?
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Yeah, would be nice to get that fixed in the kernel before we have to
cringe forever.

[-- 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

[PATCH 3/3] intel: Add a function for the new register read ioctl.

[Eric Anholt]

---
I'm not sure if this is the API we want or not.  Getting your value
back through an out parameter sucks.  On the other hand, the 3d driver
wants to be able to use the error value to detect a supported kernel.

My other API thought would be for reg_read() to return the value and
whine if an error occurs, and to have a separate can_reg_read() call.

 intel/intel_bufmgr.h     |    3 +++
 intel/intel_bufmgr_gem.c |   18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 2167e43..8d7f239 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -241,6 +241,9 @@ void drm_intel_decode_set_head_tail(struct drm_intel_decode *ctx,
 void drm_intel_decode_set_output_file(struct drm_intel_decode *ctx, FILE *out);
 void drm_intel_decode(struct drm_intel_decode *ctx);
 
+int drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
+		       uint32_t offset,
+		       uint64_t *result);
 
 /** @{ Compatibility defines to keep old code building despite the symbol rename
  * from dri_* to drm_intel_*
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index a484b12..a170813 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -2947,6 +2947,24 @@ drm_intel_gem_context_destroy(drm_intel_context *ctx)
 	free(ctx);
 }
 
+int
+drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
+		   uint32_t offset,
+		   uint64_t *result)
+{
+	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+	struct drm_i915_reg_read reg_read;
+	int ret;
+
+	VG_CLEAR(reg_read);
+	reg_read.offset = offset;
+
+	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_REG_READ, &reg_read);
+
+	*result = reg_read.val;
+	return ret;
+}
+
 
 /**
  * Annotate the given bo for use in aub dumping.
-- 
1.7.10.4

Re: [PATCH 3/3] intel: Add a function for the new register read ioctl.

[Ben Widawsky]

On 2012-08-02 11:29, Eric Anholt wrote:
> ---
> I'm not sure if this is the API we want or not.  Getting your value
> back through an out parameter sucks.  On the other hand, the 3d 
> driver
> wants to be able to use the error value to detect a supported kernel.
>
> My other API thought would be for reg_read() to return the value and
> whine if an error occurs, and to have a separate can_reg_read() call.

I'd like reg_read to just do what we want. I think making ugly code for
backward compatibility is less than ideal. OTOH, mesa and whomever else
could just wrap this in a function that does what they want. 
Furthermore,
we can just supplement a macro to also do what we want. So I think this
approach is appropriate.

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

>
>  intel/intel_bufmgr.h     |    3 +++
>  intel/intel_bufmgr_gem.c |   18 ++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> index 2167e43..8d7f239 100644
> --- a/intel/intel_bufmgr.h
> +++ b/intel/intel_bufmgr.h
> @@ -241,6 +241,9 @@ void drm_intel_decode_set_head_tail(struct
> drm_intel_decode *ctx,
>  void drm_intel_decode_set_output_file(struct drm_intel_decode *ctx,
> FILE *out);
>  void drm_intel_decode(struct drm_intel_decode *ctx);
>
> +int drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
> +		       uint32_t offset,
> +		       uint64_t *result);
>
>  /** @{ Compatibility defines to keep old code building despite the
> symbol rename
>   * from dri_* to drm_intel_*
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index a484b12..a170813 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -2947,6 +2947,24 @@ 
> drm_intel_gem_context_destroy(drm_intel_context *ctx)
>  	free(ctx);
>  }
>
> +int
> +drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
> +		   uint32_t offset,
> +		   uint64_t *result)
> +{
> +	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
> +	struct drm_i915_reg_read reg_read;
> +	int ret;
> +
> +	VG_CLEAR(reg_read);
> +	reg_read.offset = offset;
> +
> +	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_REG_READ, &reg_read);
> +
> +	*result = reg_read.val;
> +	return ret;
> +}
> +
>
>  /**
>   * Annotate the given bo for use in aub dumping.

Re: [PATCH 1/3] Drop "-Wunsafe-loop-optimizations".

[Ben Widawsky]

On 2012-08-02 11:29, Eric Anholt wrote:
> It warns about totally sensible things done in intel_decode.c.  I've
> never seen this warn do anything useful, and apparently I was the one
> to introduce it when I added the giant pile of warning flags back in
> 2008.
> ---
>  configure.ac |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 09fed53..3eaec74 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -133,7 +133,7 @@ MAYBE_WARN="-Wall -Wextra \
>  -Wpointer-arith -Wwrite-strings -Wstrict-prototypes \
>  -Wmissing-prototypes -Wmissing-declarations -Wnested-externs \
>  -Wpacked -Wswitch-enum -Wmissing-format-attribute \
> --Wstrict-aliasing=2 -Winit-self -Wunsafe-loop-optimizations \
> +-Wstrict-aliasing=2 -Winit-self \
>  -Wdeclaration-after-statement -Wold-style-definition \
>  -Wno-missing-field-initializers -Wno-unused-parameter \
>  -Wno-attributes -Wno-long-long -Winline"

If you're interested:
http://lists.freedesktop.org/archives/dri-devel/2012-June/024636.html