On 06/07/2020 07:19, Chris Wilson wrote:
The prospect of locking the entire submission sequence under a wide
ww_mutex re-imposes some key restrictions, in particular that we must
not call copy_(from|to)_user underneath the mutex (as the faulthandlers
themselves may need to take the ww_mutex). To satisfy this requirement,
we need to split the relocation handling into multiple phases again.
After dropping the reservations, we need to allocate enough buffer space
to both copy the relocations from userspace into, and serve as the
relocation command buffer. Once we have finished copying the
relocations, we can then re-aquire all the objects for the execbuf and
rebind them, including our new relocations objects. After we have bound
all the new and old objects into their final locations, we can then
convert the relocation entries into the GPU commands to update the
relocated vma. Finally, once it is all over and we have dropped the
ww_mutex for the last time, we can then complete the update of the user
relocation entries.
Good text. :)
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 842 ++++++++----------
.../i915/gem/selftests/i915_gem_execbuffer.c | 195 ++--
2 files changed, 520 insertions(+), 517 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 629b736adf2c..fbf5c5cd51ca 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -35,6 +35,7 @@ struct eb_vma {
/** This vma's place in the execbuf reservation list */
struct drm_i915_gem_exec_object2 *exec;
+ u32 bias;
struct list_head bind_link;
struct list_head unbound_link;
@@ -60,15 +61,12 @@ struct eb_vma_array {
#define __EXEC_OBJECT_HAS_PIN BIT(31)
#define __EXEC_OBJECT_HAS_FENCE BIT(30)
#define __EXEC_OBJECT_NEEDS_MAP BIT(29)
-#define __EXEC_OBJECT_NEEDS_BIAS BIT(28)
-#define __EXEC_OBJECT_INTERNAL_FLAGS (~0u << 28) /* all of the above */
+#define __EXEC_OBJECT_INTERNAL_FLAGS (~0u << 29) /* all of the above */
#define __EXEC_HAS_RELOC BIT(31)
#define __EXEC_INTERNAL_FLAGS (~0u << 31)
#define UPDATE PIN_OFFSET_FIXED
-#define BATCH_OFFSET_BIAS (256*1024)
-
#define __I915_EXEC_ILLEGAL_FLAGS \
(__I915_EXEC_UNKNOWN_FLAGS | \
I915_EXEC_CONSTANTS_MASK | \
@@ -266,20 +264,18 @@ struct i915_execbuffer {
* obj/page
*/
struct reloc_cache {
- struct drm_mm_node node; /** temporary GTT binding */
unsigned int gen; /** Cached value of INTEL_GEN */
bool use_64bit_reloc : 1;
- bool has_llc : 1;
bool has_fence : 1;
bool needs_unfenced : 1;
struct intel_context *ce;
- struct i915_vma *target;
- struct i915_request *rq;
- struct i915_vma *rq_vma;
- u32 *rq_cmd;
- unsigned int rq_size;
+ struct eb_relocs_link {
+ struct i915_vma *vma;
+ } head;
+ struct drm_i915_gem_relocation_entry *map;
+ unsigned int pos;
It's not trivial so please add some commentary around the new struct
members. List handling (is there a single linked list in kernel which
could be used for clarity?). Or maybe it is not a list.. Why is a list
of vma links inside a mapped GEM bo? What are pos, bufsz, later max,
etc. So a bit of high level operation and a bit of per field. I think
it's needed because it is not straightforward.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
