Re: [Intel-gfx] [PATCH 15/53] drm/i915: Update i915_gem_object_sync() to take a request structure

2015-03-05 Thread Tomas Elf

On 19/02/2015 17:17, john.c.harri...@intel.com wrote:

From: John Harrison john.c.harri...@intel.com

The plan is to pass requests around as the basic submission tracking structure
rather than rings and contexts. This patch updates the i915_gem_object_sync()
code path.

For: VIZ-5115
Signed-off-by: John Harrison john.c.harri...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.h|2 +-
  drivers/gpu/drm/i915/i915_gem.c|7 ---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
  drivers/gpu/drm/i915/intel_lrc.c   |2 +-
  4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 375d4f9..bfd7b47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2744,7 +2744,7 @@ static inline void i915_gem_object_unpin_pages(struct 
drm_i915_gem_object *obj)

  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
  int i915_gem_object_sync(struct drm_i915_gem_object *obj,
-struct intel_engine_cs *to);
+struct drm_i915_gem_request *to_req);
  void i915_vma_move_to_active(struct i915_vma *vma,
 struct intel_engine_cs *ring);
  int i915_gem_dumb_create(struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5897d54..c5b9bc7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2956,7 +2956,7 @@ out:
   * i915_gem_object_sync - sync an object to a ring.
   *
   * @obj: object which may be in use on another ring.
- * @to: ring we wish to use the object on. May be NULL.
+ * @to_req: request we wish to use the object for. May be NULL.
   *
   * This code is meant to abstract object synchronization with the GPU.
   * Calling with NULL implies synchronizing the object with the CPU
@@ -2966,8 +2966,9 @@ out:
   */
  int
  i915_gem_object_sync(struct drm_i915_gem_object *obj,
-struct intel_engine_cs *to)
+struct drm_i915_gem_request *to_req)
  {
+   struct intel_engine_cs *to = to_req ? to_req-ring : NULL;
struct intel_engine_cs *from;
u32 seqno;
int ret, idx;
@@ -3953,7 +3954,7 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
if (ret)
return ret;

-   ret = i915_gem_object_sync(obj, req-ring);
+   ret = i915_gem_object_sync(obj, req);
if (ret)
return ret;

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 76f6dcf..2cd0579 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -838,7 +838,7 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request 
*req,

list_for_each_entry(vma, vmas, exec_list) {
struct drm_i915_gem_object *obj = vma-obj;
-   ret = i915_gem_object_sync(obj, req-ring);
+   ret = i915_gem_object_sync(obj, req);
if (ret)
return ret;

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4b42346..0d88e9c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -589,7 +589,7 @@ static int execlists_move_to_gpu(struct 
drm_i915_gem_request *req,
list_for_each_entry(vma, vmas, exec_list) {
struct drm_i915_gem_object *obj = vma-obj;

-   ret = i915_gem_object_sync(obj, req-ring);
+   ret = i915_gem_object_sync(obj, req);
if (ret)
return ret;




This seems to be just fine but if the requests of any of the existing 
i915_gem_object_sync() call sites were to be null they would cause local 
null pointer exceptions in the calling functions even though 
i915_gem_object_sync() itself supports an incoming null request. There 
does not seem to be any call sites in the driver that actually passes or 
supports passing null requests into i915_gem_object_sync(). Does the 
driver do that anywhere today or how does that fit in?


Aside from that open question:

Reviewed-by: Tomas Elf tomas@intel.com

Thanks,
Tomas

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


[Intel-gfx] [PATCH 15/53] drm/i915: Update i915_gem_object_sync() to take a request structure

2015-02-19 Thread John . C . Harrison
From: John Harrison john.c.harri...@intel.com

The plan is to pass requests around as the basic submission tracking structure
rather than rings and contexts. This patch updates the i915_gem_object_sync()
code path.

For: VIZ-5115
Signed-off-by: John Harrison john.c.harri...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|2 +-
 drivers/gpu/drm/i915/i915_gem.c|7 ---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
 drivers/gpu/drm/i915/intel_lrc.c   |2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 375d4f9..bfd7b47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2744,7 +2744,7 @@ static inline void i915_gem_object_unpin_pages(struct 
drm_i915_gem_object *obj)
 
 int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
 int i915_gem_object_sync(struct drm_i915_gem_object *obj,
-struct intel_engine_cs *to);
+struct drm_i915_gem_request *to_req);
 void i915_vma_move_to_active(struct i915_vma *vma,
 struct intel_engine_cs *ring);
 int i915_gem_dumb_create(struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5897d54..c5b9bc7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2956,7 +2956,7 @@ out:
  * i915_gem_object_sync - sync an object to a ring.
  *
  * @obj: object which may be in use on another ring.
- * @to: ring we wish to use the object on. May be NULL.
+ * @to_req: request we wish to use the object for. May be NULL.
  *
  * This code is meant to abstract object synchronization with the GPU.
  * Calling with NULL implies synchronizing the object with the CPU
@@ -2966,8 +2966,9 @@ out:
  */
 int
 i915_gem_object_sync(struct drm_i915_gem_object *obj,
-struct intel_engine_cs *to)
+struct drm_i915_gem_request *to_req)
 {
+   struct intel_engine_cs *to = to_req ? to_req-ring : NULL;
struct intel_engine_cs *from;
u32 seqno;
int ret, idx;
@@ -3953,7 +3954,7 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
if (ret)
return ret;
 
-   ret = i915_gem_object_sync(obj, req-ring);
+   ret = i915_gem_object_sync(obj, req);
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 76f6dcf..2cd0579 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -838,7 +838,7 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request 
*req,
 
list_for_each_entry(vma, vmas, exec_list) {
struct drm_i915_gem_object *obj = vma-obj;
-   ret = i915_gem_object_sync(obj, req-ring);
+   ret = i915_gem_object_sync(obj, req);
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4b42346..0d88e9c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -589,7 +589,7 @@ static int execlists_move_to_gpu(struct 
drm_i915_gem_request *req,
list_for_each_entry(vma, vmas, exec_list) {
struct drm_i915_gem_object *obj = vma-obj;
 
-   ret = i915_gem_object_sync(obj, req-ring);
+   ret = i915_gem_object_sync(obj, req);
if (ret)
return ret;
 
-- 
1.7.9.5

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