diff mbox

[RFC] drm/i195: Add flag to enable virtual mappings above 4Gb

Message ID 1426697873-14389-1-git-send-email-nicholas.hoath@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nick Hoath March 18, 2015, 4:57 p.m. UTC
Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset hardware
workarounds require that GeneralStateOffset & InstructionBaseOffset
are restricted to a 32 bit address space.

This is a preparatory patch prior to supporting 64bit virtual memory
allocations.

Allow the user space to flag that a mapping can occur beyond
the 32bit limit. This allows backward compatibility and user space
drivers that haven't been enhanced to support these workarounds to
function.

Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
 drivers/gpu/drm/i915/i915_gem.c | 18 +++++++++++++++---
 include/uapi/drm/i915_drm.h     |  7 ++++++-
 3 files changed, 27 insertions(+), 4 deletions(-)

Comments

Tvrtko Ursulin March 18, 2015, 5:08 p.m. UTC | #1
On 03/18/2015 04:57 PM, Nick Hoath wrote:
> Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset hardware
> workarounds require that GeneralStateOffset & InstructionBaseOffset
> are restricted to a 32 bit address space.
>
> This is a preparatory patch prior to supporting 64bit virtual memory
> allocations.
>
> Allow the user space to flag that a mapping can occur beyond
> the 32bit limit. This allows backward compatibility and user space
> drivers that haven't been enhanced to support these workarounds to
> function.

Just on the naming..

Maybe the whole thing would better be named "full mem", "full range" or 
something like that?

"Hi mem", especially since the name is used in the userspace API has so 
much historical baggage meaning it needs to be _only_ in the special 
high memory range.

Regards,

Tvrtko
Daniel Vetter March 19, 2015, 4:29 p.m. UTC | #2
On Wed, Mar 18, 2015 at 04:57:53PM +0000, Nick Hoath wrote:
> Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset hardware
> workarounds require that GeneralStateOffset & InstructionBaseOffset
> are restricted to a 32 bit address space.
> 
> This is a preparatory patch prior to supporting 64bit virtual memory
> allocations.
> 
> Allow the user space to flag that a mapping can occur beyond
> the 32bit limit. This allows backward compatibility and user space
> drivers that haven't been enhanced to support these workarounds to
> function.
> 
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>

Because of the libdrm buffer cache being opaque to the different users
I don't see how a flag set at create time will work.

Also on a very quick look all userspace is still nice and relocates
indirect state with I915_GEM_DOMAIN_INSTRUCTION (well except no-reloc
userspace where this is lost). Can't we just piggy-pack on top of that?
Creative abuse of established abi ;-) But that's easy to resolve with an
additional bit for no-reloc in execbuf to signal that it's indirect state
and one flag to allow it.

Upside of that approach is that we don't need to change anything in
userspace, except SNA (atm the only no-reloc user).

And yeah I think for this one here we need to have all the support rolled
out for everyone to make sure it actually works ...

Cheers, Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3cc0196..1e6fc1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2066,6 +2066,12 @@  struct drm_i915_gem_object {
 	unsigned int has_dma_mapping:1;
 
 	unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;
+
+	/**
+	 * If the object should be mapped in to the bottom 4Gb
+	 * memory space only, then this flag should not be set
+	 */
+	unsigned int hi_mem:1;
 
 	struct sg_table *pages;
 	int pages_pin_count;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 61134ab..efa782c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -395,7 +395,9 @@  static int
 i915_gem_create(struct drm_file *file,
 		struct drm_device *dev,
 		uint64_t size,
-		uint32_t *handle_p)
+		uint32_t *handle_p,
+		uint32_t flags
+		)
 {
 	struct drm_i915_gem_object *obj;
 	int ret;
@@ -410,6 +412,9 @@  i915_gem_create(struct drm_file *file,
 	if (obj == NULL)
 		return -ENOMEM;
 
+	if (flags & I915_CREATE_FLAG_HI_MEM)
+		obj->hi_mem = 1;
+
 	ret = drm_gem_handle_create(file, &obj->base, &handle);
 	/* drop reference from allocate - handle holds it now */
 	drm_gem_object_unreference_unlocked(&obj->base);
@@ -429,7 +434,8 @@  i915_gem_dumb_create(struct drm_file *file,
 	args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64);
 	args->size = args->pitch * args->height;
 	return i915_gem_create(file, dev,
-			       args->size, &args->handle);
+			       args->size, &args->handle,
+			       I915_CREATE_FLAG_HI_MEM);
 }
 
 /**
@@ -440,9 +446,10 @@  i915_gem_create_ioctl(struct drm_device *dev, void *data,
 		      struct drm_file *file)
 {
 	struct drm_i915_gem_create *args = data;
 
 	return i915_gem_create(file, dev,
-			       args->size, &args->handle);
+			       args->size, &args->handle,
+			       args->flags);
 }
 
 static inline int
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6eed16b..eb2e7d9 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -428,6 +428,8 @@  struct drm_i915_gem_init {
 	__u64 gtt_end;
 };
 
+#define I915_CREATE_FLAG_HI_MEM	0x00000001
+
 struct drm_i915_gem_create {
 	/**
 	 * Requested size for the object.
@@ -441,7 +443,10 @@  struct drm_i915_gem_create {
 	 * Object handles are nonzero.
 	 */
 	__u32 handle;
-	__u32 pad;
+	/**
+	 * Object creation flags
+	 */
+	__u32 flags;
 };
 
 struct drm_i915_gem_pread {