Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Wed, Sep 11, 2013 at 02:57:53PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm-bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. Oh, this is disappointing. The GLOBAL_BIND flag is a let down. Is there any way we can have the aliasing_ppgtt as a separate vm and so keep the purity? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Tue, Sep 03, 2013 at 05:20:08PM -0700, Ben Widawsky wrote:
On Mon, Sep 02, 2013 at 03:46:52PM +0300, Ville Syrjälä wrote:
On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote:
From: Ben Widawsky b...@bwidawsk.net
As we plumb the code with more VM information, it has become more
obvious that the easiest way to deal with bind and unbind is to simply
put the function pointers in the vm, and let those choose the correct
way to handle the page table updates. This change allows many places in
the code to simply be vm-bind, and not have to worry about
distinguishing PPGTT vs GGTT.
Notice that this patch has no impact on functionality. I've decided to
save the actual change until the next patch because I think it's easier
to review that way. I'm happy to squash the two, or let Daniel do it on
merge.
v2:
Make ggtt handle the quirky aliasing ppgtt
Add flags to bind object to support above
Don't ever call bind/unbind directly for PPGTT until we have real, full
PPGTT (use NULLs to assert this)
Make sure we rebind the ggtt if there already is a ggtt binding. This
happens on set cache levels
Use VMA for bind/unbind (Daniel, Ben)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 69 +---
drivers/gpu/drm/i915/i915_gem_gtt.c | 101
2 files changed, 140 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index c9ed77a..377ca63 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -461,6 +461,36 @@ enum i915_cache_level {
typedef uint32_t gen6_gtt_pte_t;
+/**
+ * A VMA represents a GEM BO that is bound into an address space.
Therefore, a
+ * VMA's presence cannot be guaranteed before binding, or after
unbinding the
+ * object into/from the address space.
+ *
+ * To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
+ * will always be = an objects lifetime. So object refcounting should
cover us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ /** This object's place on the active/inactive lists */
+ struct list_head mm_list;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+
+ /** This vma's place in the batchbuffer or on the eviction list */
+ struct list_head exec_list;
+
+ /**
+ * Used for performing relocations during execbuffer insertion.
+ */
+ struct hlist_node exec_node;
+ unsigned long exec_handle;
+ struct drm_i915_gem_exec_object2 *exec_entry;
+
+};
+
struct i915_address_space {
struct drm_mm mm;
struct drm_device *dev;
@@ -499,9 +529,18 @@ struct i915_address_space {
/* FIXME: Need a more generic return type */
gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
enum i915_cache_level level);
+
+ /** Unmap an object from an address space. This usually consists of
+ * setting the valid PTE entries to a reserved scratch page. */
+ void (*unbind_vma)(struct i915_vma *vma);
void (*clear_range)(struct i915_address_space *vm,
unsigned int first_entry,
unsigned int num_entries);
+ /* Map an object into an address space with the given cache flags. */
+#define GLOBAL_BIND (10)
+ void (*bind_vma)(struct i915_vma *vma,
+ enum i915_cache_level cache_level,
+ u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st,
unsigned int first_entry,
@@ -548,36 +587,6 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
-/**
- * A VMA represents a GEM BO that is bound into an address space.
Therefore, a
- * VMA's presence cannot be guaranteed before binding, or after
unbinding the
- * object into/from the address space.
- *
- * To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
- * will always be = an objects lifetime. So object refcounting should
cover us.
- */
-struct i915_vma {
- struct drm_mm_node node;
- struct drm_i915_gem_object *obj;
- struct i915_address_space *vm;
-
- /** This object's place on the active/inactive lists */
- struct list_head mm_list;
-
- struct list_head vma_link; /* Link in the object's VMA list */
-
- /** This vma's place in the batchbuffer or on the eviction list */
- struct list_head exec_list;
-
- /**
- * Used for performing relocations during execbuffer insertion.
- */
- struct hlist_node exec_node;
- unsigned long exec_handle;
- struct
Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Fri, Aug 30, 2013 at 08:40:36PM -0700, Ben Widawsky wrote: On Sat, Aug 31, 2013 at 01:12:55AM +0100, Chris Wilson wrote: On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm-bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels Use VMA for bind/unbind (Daniel, Ben) Signed-off-by: Ben Widawsky b...@bwidawsk.net You like pokkadot paint for its inconsistency? Other than interesting alternation of styles, I see nothing wrong with the logic. -Chris To what are you referring? I'm probably more than willing to change whatever displeases you. You were alternating between using temporary variables like const unsigned long entry = vma-node.start PAGE_SHIFT; and doing the computation inline, with no consistency as far as I could see. It was so very minor, but it looks like cut'n'paste code from multiple sources. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Mon, Sep 02, 2013 at 03:46:52PM +0300, Ville Syrjälä wrote:
On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote:
From: Ben Widawsky b...@bwidawsk.net
As we plumb the code with more VM information, it has become more
obvious that the easiest way to deal with bind and unbind is to simply
put the function pointers in the vm, and let those choose the correct
way to handle the page table updates. This change allows many places in
the code to simply be vm-bind, and not have to worry about
distinguishing PPGTT vs GGTT.
Notice that this patch has no impact on functionality. I've decided to
save the actual change until the next patch because I think it's easier
to review that way. I'm happy to squash the two, or let Daniel do it on
merge.
v2:
Make ggtt handle the quirky aliasing ppgtt
Add flags to bind object to support above
Don't ever call bind/unbind directly for PPGTT until we have real, full
PPGTT (use NULLs to assert this)
Make sure we rebind the ggtt if there already is a ggtt binding. This
happens on set cache levels
Use VMA for bind/unbind (Daniel, Ben)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 69 +---
drivers/gpu/drm/i915/i915_gem_gtt.c | 101
2 files changed, 140 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index c9ed77a..377ca63 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -461,6 +461,36 @@ enum i915_cache_level {
typedef uint32_t gen6_gtt_pte_t;
+/**
+ * A VMA represents a GEM BO that is bound into an address space.
Therefore, a
+ * VMA's presence cannot be guaranteed before binding, or after unbinding
the
+ * object into/from the address space.
+ *
+ * To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
+ * will always be = an objects lifetime. So object refcounting should
cover us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ /** This object's place on the active/inactive lists */
+ struct list_head mm_list;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+
+ /** This vma's place in the batchbuffer or on the eviction list */
+ struct list_head exec_list;
+
+ /**
+* Used for performing relocations during execbuffer insertion.
+*/
+ struct hlist_node exec_node;
+ unsigned long exec_handle;
+ struct drm_i915_gem_exec_object2 *exec_entry;
+
+};
+
struct i915_address_space {
struct drm_mm mm;
struct drm_device *dev;
@@ -499,9 +529,18 @@ struct i915_address_space {
/* FIXME: Need a more generic return type */
gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
enum i915_cache_level level);
+
+ /** Unmap an object from an address space. This usually consists of
+* setting the valid PTE entries to a reserved scratch page. */
+ void (*unbind_vma)(struct i915_vma *vma);
void (*clear_range)(struct i915_address_space *vm,
unsigned int first_entry,
unsigned int num_entries);
+ /* Map an object into an address space with the given cache flags. */
+#define GLOBAL_BIND (10)
+ void (*bind_vma)(struct i915_vma *vma,
+enum i915_cache_level cache_level,
+u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st,
unsigned int first_entry,
@@ -548,36 +587,6 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
-/**
- * A VMA represents a GEM BO that is bound into an address space.
Therefore, a
- * VMA's presence cannot be guaranteed before binding, or after unbinding
the
- * object into/from the address space.
- *
- * To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
- * will always be = an objects lifetime. So object refcounting should
cover us.
- */
-struct i915_vma {
- struct drm_mm_node node;
- struct drm_i915_gem_object *obj;
- struct i915_address_space *vm;
-
- /** This object's place on the active/inactive lists */
- struct list_head mm_list;
-
- struct list_head vma_link; /* Link in the object's VMA list */
-
- /** This vma's place in the batchbuffer or on the eviction list */
- struct list_head exec_list;
-
- /**
-* Used for performing relocations during execbuffer insertion.
-*/
- struct hlist_node exec_node;
- unsigned long exec_handle;
- struct drm_i915_gem_exec_object2 *exec_entry;
-
-};
-
struct i915_ctx_hang_stats {
/* This context had
Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote:
From: Ben Widawsky b...@bwidawsk.net
As we plumb the code with more VM information, it has become more
obvious that the easiest way to deal with bind and unbind is to simply
put the function pointers in the vm, and let those choose the correct
way to handle the page table updates. This change allows many places in
the code to simply be vm-bind, and not have to worry about
distinguishing PPGTT vs GGTT.
Notice that this patch has no impact on functionality. I've decided to
save the actual change until the next patch because I think it's easier
to review that way. I'm happy to squash the two, or let Daniel do it on
merge.
v2:
Make ggtt handle the quirky aliasing ppgtt
Add flags to bind object to support above
Don't ever call bind/unbind directly for PPGTT until we have real, full
PPGTT (use NULLs to assert this)
Make sure we rebind the ggtt if there already is a ggtt binding. This
happens on set cache levels
Use VMA for bind/unbind (Daniel, Ben)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 69 +---
drivers/gpu/drm/i915/i915_gem_gtt.c | 101
2 files changed, 140 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9ed77a..377ca63 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -461,6 +461,36 @@ enum i915_cache_level {
typedef uint32_t gen6_gtt_pte_t;
+/**
+ * A VMA represents a GEM BO that is bound into an address space. Therefore,
a
+ * VMA's presence cannot be guaranteed before binding, or after unbinding the
+ * object into/from the address space.
+ *
+ * To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
+ * will always be = an objects lifetime. So object refcounting should cover
us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ /** This object's place on the active/inactive lists */
+ struct list_head mm_list;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+
+ /** This vma's place in the batchbuffer or on the eviction list */
+ struct list_head exec_list;
+
+ /**
+ * Used for performing relocations during execbuffer insertion.
+ */
+ struct hlist_node exec_node;
+ unsigned long exec_handle;
+ struct drm_i915_gem_exec_object2 *exec_entry;
+
+};
+
struct i915_address_space {
struct drm_mm mm;
struct drm_device *dev;
@@ -499,9 +529,18 @@ struct i915_address_space {
/* FIXME: Need a more generic return type */
gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
enum i915_cache_level level);
+
+ /** Unmap an object from an address space. This usually consists of
+ * setting the valid PTE entries to a reserved scratch page. */
+ void (*unbind_vma)(struct i915_vma *vma);
void (*clear_range)(struct i915_address_space *vm,
unsigned int first_entry,
unsigned int num_entries);
+ /* Map an object into an address space with the given cache flags. */
+#define GLOBAL_BIND (10)
+ void (*bind_vma)(struct i915_vma *vma,
+ enum i915_cache_level cache_level,
+ u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st,
unsigned int first_entry,
@@ -548,36 +587,6 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
-/**
- * A VMA represents a GEM BO that is bound into an address space. Therefore,
a
- * VMA's presence cannot be guaranteed before binding, or after unbinding the
- * object into/from the address space.
- *
- * To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
- * will always be = an objects lifetime. So object refcounting should cover
us.
- */
-struct i915_vma {
- struct drm_mm_node node;
- struct drm_i915_gem_object *obj;
- struct i915_address_space *vm;
-
- /** This object's place on the active/inactive lists */
- struct list_head mm_list;
-
- struct list_head vma_link; /* Link in the object's VMA list */
-
- /** This vma's place in the batchbuffer or on the eviction list */
- struct list_head exec_list;
-
- /**
- * Used for performing relocations during execbuffer insertion.
- */
- struct hlist_node exec_node;
- unsigned long exec_handle;
- struct drm_i915_gem_exec_object2 *exec_entry;
-
-};
-
struct i915_ctx_hang_stats {
/* This context had batch pending when hang was declared */
unsigned batch_pending;
diff --git
Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm-bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels Use VMA for bind/unbind (Daniel, Ben) Signed-off-by: Ben Widawsky b...@bwidawsk.net You like pokkadot paint for its inconsistency? Other than interesting alternation of styles, I see nothing wrong with the logic. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM
On Sat, Aug 31, 2013 at 01:12:55AM +0100, Chris Wilson wrote: On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm-bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels Use VMA for bind/unbind (Daniel, Ben) Signed-off-by: Ben Widawsky b...@bwidawsk.net You like pokkadot paint for its inconsistency? Other than interesting alternation of styles, I see nothing wrong with the logic. -Chris To what are you referring? I'm probably more than willing to change whatever displeases you. -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
